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 4b0f2e31f..e124002f6 100644 --- a/test/packetimpact/tests/tcp_listen_backlog_test.go +++ b/test/packetimpact/tests/tcp_listen_backlog_test.go @@ -150,17 +150,10 @@ func TestTCPListenBacklog(t *testing.T) { 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 !dut.Uname.IsLinux() { - // TODO(https://gvisor.dev/issues/6683): Expect a RST. - if got, err := conn.Expect(t, testbench.TCP{}, time.Second); err == nil { - t.Errorf("expected no TCP frame, got %s", got) - } - } else { - 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) - } + 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) } }()