google / google/gvisor

IPTables: `-j REJECT` defaults to reject with icmp-port-unreachable and it deadlocks on connect

Open
#13,756 3 comments 0 reactions 1 assignee Claimed by @parth-opensrc View on GitHub
type: bug
Dominant language
Go
Stars
19.3k
Forks
2k
Avg merge
3d 5h
Merged PRs (30d)
264

Description

### Description

#13562 added `-j REJECT` support to netstack iptables. It claims to support `tcp-reset` mainly but it seems like the code itself tried to support `--reject-with icmp-port-unreachable` and actually made it the default for `-j REJECT` like Linux.

However, on the Output hook the rejected packet was locally generated, so the ICMP error loops straight back into the stack and is destined to the originating endpoint. Delivering it inline reaches (*tcp.Endpoint).onICMPError from the goroutine performing Connect, which still holds the endpoint lock. onICMPError would then try to acquire the same non-reentrant lock again. Which deadlocks.

Consider the following netstack test (disclaimer: it's slop. i skimmed over it and it makes sense):

```
func TestRejectWithICMPOutputHookTCPConnect(t *testing.T) {
const (
nicID = 1
port = 12345
// Locally-generated packets hit the Output hook synchronously, so
// Connect returns immediately once the bug is fixed; a multi-second
// wait only ever fires when the endpoint deadlocked.
deadlockTimeout = 5 * time.Second
)

tests := []struct {
name string
netProto tcpip.NetworkProtocolNumber
ipv6 bool
protoAddr tcpip.ProtocolAddress
rejectTarget func(t *testing.T, s *stack.Stack) stack.Target
}{
{
name: "IPv4",
netProto: ipv4.ProtocolNumber,
ipv6: false,
protoAddr: tcpip.ProtocolAddress{
Protocol: ipv4.ProtocolNumber,
AddressWithPrefix: utils.Ipv4Addr,
},
rejectTarget: func(t *testing.T, s *stack.Stack) stack.Target {
t.Helper()
netProto := s.NetworkProtocolInstance(header.IPv4ProtocolNumber)
handler, ok := netProto.(stack.RejectIPv4WithHandler)
if !ok {
t.Fatalf("got %T, want it to implement stack.RejectIPv4WithHandler", netProto)
}
return &stack.RejectIPv4Target{
Handler: handler,
RejectWith: stack.RejectIPv4WithICMPPortUnreachable,
}
},
},
{
name: "IPv6",
netProto: ipv6.ProtocolNumber,
ipv6: true,
protoAddr: tcpip.ProtocolAddress{
Protocol: ipv6.ProtocolNumber,
AddressWithPrefix: utils.Ipv6Addr,
},
rejectTarget: func(t *testing.T, s *stack.Stack) stack.Target {
t.Helper()
netProto := s.NetworkProtocolInstance(header.IPv6ProtocolNumber)
handler, ok := netProto.(stack.RejectIPv6WithHandler)
if !ok {
t.Fatalf("got %T, want it to implement stack.RejectIPv6WithHandler", netProto)
}
return &stack.RejectIPv6Target{
Handler: handler,
RejectWith: stack.RejectIPv6WithICMPPortUnreachable,
}
},
},
}

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
s := stack.New(stack.Options{
NetworkProtocols: []stack.NetworkProtocolFactory{ipv4.NewProtocol, ipv6.NewProtocol},
TransportProtocols: []stack.TransportProtocolFactory{tcp.NewProtocol},
})
if err := s.CreateNIC(nicID, loopback.New()); err != nil {
t.Fatalf("s.CreateNIC(%d, _): %s", nicID, err)
}
if err := s.AddProtocolAddress(nicID, test.protoAddr, stack.AddressProperties{}); err != nil {
t.Fatalf("s.AddProtocolAddress(%d, %+v, {}): %s", nicID, test.protoAddr, err)
}
s.SetRouteTable([]tcpip.Route{
{Destination: header.IPv4EmptySubnet, NIC: nicID},
{Destination: header.IPv6EmptySubnet, NIC: nicID},
})

// Reject TCP packets on the Output hook with an ICMP port
// unreachable error. Only TCP is matched so that the generated
// ICMP error itself is allowed out and delivered back to the
// connecting endpoint.
ipt := s.IPTables()
table := ipt.GetTable(stack.FilterID, test.ipv6)
ruleIdx := table.BuiltinChains[stack.Output]
table.Rules[ruleIdx].Filter = stack.IPHeaderFilter{
Protocol: header.TCPProtocolNumber,
CheckProtocol: true,
}
table.Rules[ruleIdx].Target = test.rejectTarget(t, s)
// Make sure the packet is not dropped by the next rule.
table.Rules[ruleIdx+1].Target = &stack.AcceptTarget{}
ipt.ForceReplaceTable(stack.FilterID, table, test.ipv6)

var listenerWQ waiter.Queue
listenerEP, err := s.NewEndpoint(tcp.ProtocolNumber, test.netProto, &listenerWQ)
if err != nil {
t.Fatalf("s.NewEndpoint(%d, %d, _): %s", tcp.ProtocolNumber, test.netProto, err)
}
if err := listenerEP.Bind(tcpip.FullAddress{Port: port}); err != nil {
t.Fatalf("listenerEP.Bind(%d): %s", port, err)
}
if err := listenerEP.Listen(1); err != nil {
t.Fatalf("listenerEP.Listen(1): %s", err)
}

var clientWQ waiter.Queue
we, ch := waiter.NewChannelEntry(waiter.EventErr | waiter.EventHUp)
clientWQ.EventRegister(&we)
defer clientWQ.EventUnregister(&we)
clientEP, err := s.NewEndpoint(tcp.ProtocolNumber, test.netProto, &clientWQ)
if err != nil {
t.Fatalf("s.NewEndpoint(%d, %d, _): %s", tcp.ProtocolNumber, test.netProto, err)
}

// When the bug described above is present, the Connect goroutine
// below deadlocks while holding the endpoint lock. Closing the
// endpoints or destroying the stack would block on that lock, so
// skip them and let the (already failed) test leak them.
deadlocked := false
defer func() {
if deadlocked {
return
}
clientEP.Close()
listenerEP.Close()
s.Destroy()
}()

connectAddr := tcpip.FullAddress{
NIC: nicID,
Addr: test.protoAddr.AddressWithPrefix.Address,
Port: port,
}
connectErr := make(chan tcpip.Error, 1)
go func() {
connectErr <- clientEP.Connect(connectAddr)
}()

select {
case err := <-connectErr:
if _, ok := err.(*tcpip.ErrConnectStarted); !ok {
t.Fatalf("got clientEP.Connect(%#v) = %s, want = %s", connectAddr, err, &tcpip.ErrConnectStarted{})
}
case <-time.After(deadlockTimeout):
deadlocked = true
t.Errorf("clientEP.Connect(%#v) did not return within %s: REJECT on the Output hook self-deadlocked the connecting endpoint", connectAddr, deadlockTimeout)
return
}

// The ICMP error is delivered asynchronously; wait for it to fail
// the handshake.
select {
case <-ch:
case <-time.After(deadlockTimeout):
t.Fatal("timed out waiting for the connecting endpoint to observe the ICMP error")
}
if _, ok := clientEP.LastError().(*tcpip.ErrConnectionRefused); !ok {
t.Fatalf("got clientEP.LastError() = %s, want = %s", clientEP.LastError(), &tcpip.ErrConnectionRefused{})
}

// The SYN was rejected on the Output hook and never reached the
// listener.
if _, _, err := listenerEP.Accept(nil); err == nil {
t.Fatal("got listenerEP.Accept(nil) = nil, want = ErrWouldBlock")
} else if _, ok := err.(*tcpip.ErrWouldBlock); !ok {
t.Fatalf("got listenerEP.Accept(nil) = %s, want = %s", err, &tcpip.ErrWouldBlock{})
}
})
}
}
```

### Steps to reproduce

The following command hangs forever:

```
docker run --rm --runtime=runsc --cap-add NET_ADMIN --cap-add NET_RAW \
gvisor.dev/images/builder:latest \
sh -c 'iptables-legacy -A OUTPUT -p tcp -j REJECT && \
timeout 10 python3 -c "import socket; s=socket.socket(); s.connect((\"127.0.0.1\",12345))"'
```

### runsc version

```shell

```

### docker version (if using docker)

```shell

```

### uname

_No response_

### kubectl (if using Kubernetes)

```shell

```

### repo state (if built from source)

release-20260706.0-25-g9cb4418672

### runsc debug logs (if available)

```shell

```

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.