diff options
| -rw-r--r-- | pkg/tcpip/stack/conntrack.go | 23 | ||||
| -rw-r--r-- | pkg/tcpip/transport/tcp/accept.go | 36 | ||||
| -rw-r--r-- | test/packetimpact/tests/tcp_listen_backlog_test.go | 21 | ||||
| -rw-r--r-- | test/syscalls/linux/socket_ipv4_udp_unbound_external_networking.cc | 8 |
4 files changed, 43 insertions, 45 deletions
diff --git a/pkg/tcpip/stack/conntrack.go b/pkg/tcpip/stack/conntrack.go index 79bc001c7..30545f634 100644 --- a/pkg/tcpip/stack/conntrack.go +++ b/pkg/tcpip/stack/conntrack.go @@ -113,7 +113,7 @@ type conn struct { // TODO(gvisor.dev/issue/5696): Support updating manipulation type. manip manipType - mu sync.Mutex `state:"nosave"` + mu sync.RWMutex `state:"nosave"` // tcb is TCB control block. It is used to keep track of states // of tcp connection. // @@ -143,8 +143,8 @@ func newConn(orig, reply tupleID, manip manipType) *conn { func (cn *conn) timedOut(now time.Time) bool { const establishedTimeout = 5 * 24 * time.Hour const defaultTimeout = 120 * time.Second - cn.mu.Lock() - defer cn.mu.Unlock() + cn.mu.RLock() + defer cn.mu.RUnlock() if cn.tcb.State() == tcpconntrack.ResultAlive { // Use the same default as Linux, which doesn't delete // established connections for 5(!) days. @@ -215,7 +215,7 @@ type ConnTrack struct { // +stateify savable type bucket struct { - mu sync.Mutex `state:"nosave"` + mu sync.RWMutex `state:"nosave"` // +checklocks:mu tuples tupleList } @@ -279,20 +279,13 @@ func (ct *ConnTrack) connForTID(tid tupleID) (*conn, direction) { now := time.Now() ct.mu.RLock() - defer ct.mu.RUnlock() bkt := &ct.buckets[bktID] - bkt.mu.Lock() - defer bkt.mu.Unlock() + ct.mu.RUnlock() - // Iterate over the tuples in a bucket, cleaning up any unused - // connections we find. + bkt.mu.RLock() + defer bkt.mu.RUnlock() for other := bkt.tuples.Front(); other != nil; other = other.Next() { - // Clean up any timed-out connections we happen to find. - if ct.reapTupleLocked(other, bktID, bkt, now) { - // The tuple expired. - continue - } - if tid == other.tupleID { + if tid == other.tupleID && !other.conn.timedOut(now) { return other.conn, other.direction } } diff --git a/pkg/tcpip/transport/tcp/accept.go b/pkg/tcpip/transport/tcp/accept.go index 95fcdc1b6..caf14b0dc 100644 --- a/pkg/tcpip/transport/tcp/accept.go +++ b/pkg/tcpip/transport/tcp/accept.go @@ -583,23 +583,6 @@ func (e *endpoint) handleListenSegment(ctx *listenContext, s *segment) tcpip.Err return nil case s.flags.Contains(header.TCPFlagAck): - // Keep hold of acceptMu until the new endpoint is in the accept queue (or - // if there is an error), to guarantee that we will keep our spot in the - // queue even if another handshake from the syn queue completes. - e.acceptMu.Lock() - if e.acceptQueue.isFull() { - // Silently drop the ack as the application can't accept - // the connection at this point. The ack will be - // retransmitted by the sender anyway and we can - // complete the connection at the time of retransmit if - // the backlog has space. - e.acceptMu.Unlock() - e.stack.Stats().TCP.ListenOverflowAckDrop.Increment() - e.stats.ReceiveErrors.ListenOverflowAckDrop.Increment() - e.stack.Stats().DroppedPackets.Increment() - return nil - } - iss := s.ackNumber - 1 irs := s.sequenceNumber - 1 @@ -615,7 +598,6 @@ func (e *endpoint) handleListenSegment(ctx *listenContext, s *segment) tcpip.Err // Validate the cookie. data, ok := ctx.isCookieValid(s.id, iss, irs) if !ok || int(data) >= len(mssTable) { - e.acceptMu.Unlock() e.stack.Stats().TCP.ListenOverflowInvalidSynCookieRcvd.Increment() e.stack.Stats().DroppedPackets.Increment() @@ -636,6 +618,24 @@ func (e *endpoint) handleListenSegment(ctx *listenContext, s *segment) tcpip.Err // ACK was received from the sender. return replyWithReset(e.stack, s, e.sendTOS, e.ttl) } + + // Keep hold of acceptMu until the new endpoint is in the accept queue (or + // if there is an error), to guarantee that we will keep our spot in the + // queue even if another handshake from the syn queue completes. + e.acceptMu.Lock() + if e.acceptQueue.isFull() { + // Silently drop the ack as the application can't accept + // the connection at this point. The ack will be + // retransmitted by the sender anyway and we can + // complete the connection at the time of retransmit if + // the backlog has space. + e.acceptMu.Unlock() + e.stack.Stats().TCP.ListenOverflowAckDrop.Increment() + e.stats.ReceiveErrors.ListenOverflowAckDrop.Increment() + e.stack.Stats().DroppedPackets.Increment() + return nil + } + e.stack.Stats().TCP.ListenOverflowSynCookieRcvd.Increment() // Create newly accepted endpoint and deliver it. rcvdSynOptions := header.TCPSynOptions{ diff --git a/test/packetimpact/tests/tcp_listen_backlog_test.go b/test/packetimpact/tests/tcp_listen_backlog_test.go index c5fd37845..e124002f6 100644 --- a/test/packetimpact/tests/tcp_listen_backlog_test.go +++ b/test/packetimpact/tests/tcp_listen_backlog_test.go @@ -144,13 +144,26 @@ func TestTCPListenBacklog(t *testing.T) { } } + // While the accept queue is still full, send an unexpected ACK from a new + // socket. The listener should reply with an RST. + func() { + conn := dut.Net.NewTCPIPv4(t, testbench.TCP{DstPort: &remotePort}, testbench.TCP{}) + defer conn.Close(t) + conn.Send(t, testbench.TCP{Flags: testbench.TCPFlags(header.TCPFlagAck)}) + if got, err := conn.Expect(t, testbench.TCP{}, time.Second); err != nil { + t.Errorf("expected TCP frame: %s", err) + } else if got, want := *got.Flags, header.TCPFlagRst; got != want { + t.Errorf("got %s, want %s", got, want) + } + }() + func() { // Now initiate a new connection when the accept queue is full. - connectingConn := dut.Net.NewTCPIPv4(t, testbench.TCP{DstPort: &remotePort}, testbench.TCP{}) - defer connectingConn.Close(t) + conn := dut.Net.NewTCPIPv4(t, testbench.TCP{DstPort: &remotePort}, testbench.TCP{}) + defer conn.Close(t) // Expect dut connection to drop the SYN. - connectingConn.Send(t, testbench.TCP{Flags: testbench.TCPFlags(header.TCPFlagSyn)}) - if got, err := connectingConn.Expect(t, testbench.TCP{}, time.Second); err == nil { + conn.Send(t, testbench.TCP{Flags: testbench.TCPFlags(header.TCPFlagSyn)}) + if got, err := conn.Expect(t, testbench.TCP{}, time.Second); err == nil { t.Fatalf("expected no TCP frame, got %s", got) } }() diff --git a/test/syscalls/linux/socket_ipv4_udp_unbound_external_networking.cc b/test/syscalls/linux/socket_ipv4_udp_unbound_external_networking.cc index c6e775b2a..d440932da 100644 --- a/test/syscalls/linux/socket_ipv4_udp_unbound_external_networking.cc +++ b/test/syscalls/linux/socket_ipv4_udp_unbound_external_networking.cc @@ -306,10 +306,6 @@ TEST_P(IPv4UDPUnboundExternalNetworkingSocketTest, TestSendUnicastOnUnbound) { // set interface or group membership. TEST_P(IPv4UDPUnboundExternalNetworkingSocketTest, TestSendMulticastSelfNoGroup) { - // FIXME(b/125485338): A group membership is not required for external - // multicast on gVisor. - SKIP_IF(IsRunningOnGvisor()); - SKIP_IF(!found_net_interfaces_); auto socket = ASSERT_NO_ERRNO_AND_VALUE(NewSocket()); @@ -434,10 +430,6 @@ TEST_P(IPv4UDPUnboundExternalNetworkingSocketTest, // Check that multicast packets won't be delivered to another socket with no // set interface or group membership. TEST_P(IPv4UDPUnboundExternalNetworkingSocketTest, TestSendMulticastNoGroup) { - // FIXME(b/125485338): A group membership is not required for external - // multicast on gVisor. - SKIP_IF(IsRunningOnGvisor()); - SKIP_IF(!found_net_interfaces_); auto sender = ASSERT_NO_ERRNO_AND_VALUE(NewSocket()); |