google / gvisor

Application Kernel for Containers
https://gvisor.dev
Apache License 2.0
15.64k stars 1.29k forks source link

tcpip: out-of-order FIN incorrectly dropped #8320

Closed adtac closed 1 month ago

adtac commented 1 year ago

Description

I noticed a possible bug in the TCP receiver. Consider the following ordering of events:

  1. (*receiver).RcvNxt = 100
  2. receive segment A: FIN, seq=150, len=0
  3. receive segment B: PSH, seq=100, len=50

When the receiver handles A, consumeSegment will return false:

https://github.com/google/gvisor/blob/2342c658c7c2ccdc1e23d642a5873c80297669de/pkg/tcpip/transport/tcp/rcv.go#L225-L227

So handleRcvdSegment will queue the segment for later processing because it's a FIN:

https://github.com/google/gvisor/blob/2342c658c7c2ccdc1e23d642a5873c80297669de/pkg/tcpip/transport/tcp/rcv.go#L481-L503

Next segment B is handled successfully and it updates r.RcvNxt to 100+50 = 150. Then we check if there are pending queued segments:

https://github.com/google/gvisor/blob/2342c658c7c2ccdc1e23d642a5873c80297669de/pkg/tcpip/transport/tcp/rcv.go#L514-L523

The if condition on line 520 is wrong AFAICT. !segSeq.Add(segLen-1).LessThan(r.RcvNxt) will evaluate to false and exit out of the loop without calling consumeSegment even though it's the right time to consume the FIN segment. This will leave the connection hanging until a retransmitted FIN arrives.

I believe there needs to be a special check for FIN on line 520 just like in line 482. I don't have much experience with gvisor, so please correct me if I'm wrong.

Steps to reproduce

No response

runsc version

No response

docker version (if using docker)

No response

uname

No response

kubectl (if using Kubernetes)

No response

repo state (if built from source)

No response

runsc debug logs (if available)

No response

github-actions[bot] commented 1 year ago

A friendly reminder that this issue had no activity for 120 days.

github-actions[bot] commented 8 months ago

A friendly reminder that this issue had no activity for 120 days.

ayushr2 commented 8 months ago

@kevinGC any updates on this? Is this stale?

github-actions[bot] commented 4 months ago

A friendly reminder that this issue had no activity for 120 days.

github-actions[bot] commented 1 month ago

This issue has been closed due to lack of activity.