buglwIP - A Lightweight TCP/IP stack - Bugs: bug #48328, Fatal RST/ACK loop caused by...

 
 

You are not allowed to post comments on this tracker with your current authentication level.

bug #48328: Fatal RST/ACK loop caused by commit "fixed bug #48170"

Submitted by:  Ambroz Bizjak <abizjak>
Submitted on:  Tue 28 Jun 2016 10:25:59 AM UTC  
 
Category: TCPSeverity: 3 - Normal
Item Group: Faulty BehaviourStatus: Fixed
Privacy: PublicAssigned to: Simon Goldschmidt <goldsimon>
Open/Closed: ClosedPlanned Release: None
lwIP version: git head

(Jump to the original submission Jump to the original submission)

Sat 20 Aug 2016 11:55:00 AM UTC, comment #8:

Hey,
Just a remark. I'm reading the TCP RFC (793) and I think the specs under Event Processing actually contradicts the more informal description on reset handling on page 37, which lwIP supposedly violates.

On page 69, acceptability of a received segment is defined, which does indeed classify out-of-sequence RSTs as acceptable. However, there is also this text:

"In the following it is assumed that the segment is the idealized segment that begins at RCV.NXT and does not exceed the window. ..."

And since the RST checking (page 70, 71) is specified after this, out-of-sequence (but in-window) RSTs should be dropped or buffered for later processing, rather than abort the connection immediately. On the other only non-acceptable (according to the exact definition on page 69) RSTs should be ACKed.

lwIP could be changed to be more compliant in that respect.

Ambroz Bizjak <abizjak>
Thu 30 Jun 2016 06:51:39 AM UTC, comment #7:

I've pushed a better fix. Thanks for reporting.

Simon Goldschmidt <goldsimon>
Project AdministratorIn charge of this item.
Thu 30 Jun 2016 06:27:00 AM UTC, comment #6:

Right, seems like I messed that up. However, I don't agree to the patch as it accepts "seqno == pcb->rcv_nxt" in stat SYN_SENT, too, which it didn't before.

Simon Goldschmidt <goldsimon>
Project AdministratorIn charge of this item.
Tue 28 Jun 2016 11:11:30 AM UTC, comment #5:

I confirm the change in comment #3 (and comment #2 ignoring the second change that is irrelevant) fixes the issue for me.

Ambroz Bizjak <abizjak>
Tue 28 Jun 2016 11:07:50 AM UTC, comment #4:

I don't agree with applying this patch, we should revert the wrong fix and apply the real fix in an atomic patch with both diffset combined.

Sylvain Rochet <gradator>
Project Member
Tue 28 Jun 2016 11:04:32 AM UTC, comment #3:

Attached the patch.

(file #37611)

Axel Lin <axellin>
Tue 28 Jun 2016 10:54:57 AM UTC, comment #2:

That should do it, I referenced this commit in this mail which talk about this commit:

http://lists.nongnu.org/archive/html/lwip-devel/2016-06/msg00053.html

Sylvain Rochet <gradator>
Project Member
Tue 28 Jun 2016 10:45:09 AM UTC, comment #1:

Does below change work for you?
I found it on https://github.com/tabascoeye/lwip/commit/a59a9c102c924cca420dfbd5063fbcfb8e560bac

diff --git a/src/core/tcp_in.c b/src/core/tcp_in.c
index b26f8e6..1822206 100644
--- a/src/core/tcp_in.c
+++ b/src/core/tcp_in.c
@@ -703,8 +703,8 @@ tcp_process(struct tcp_pcb *pcb)

/* Process incoming RST segments. */
if (flags & TCP_RST) {
- /* First, determine if the reset is acceptable. (in case of RST only if the sequence number matches) */
- if (ackno == pcb->snd_nxt) {
+ /* First, determine if the reset is acceptable. (in case of RST only if the sequence number matches, special case for SYN_SENT state) */
+ if (((pcb->state == SYN_SENT) && (ackno == pcb->snd_nxt)) || (seqno == pcb->rcv_nxt)) {
LWIP_DEBUGF(TCP_INPUT_DEBUG, ("tcp_process: Connection RESET\n"));
LWIP_ASSERT("tcp_input: pcb->state != CLOSED", pcb->state != CLOSED);
recv_flags |= TF_RESET;

Axel

Axel Lin <axellin>
Tue 28 Jun 2016 10:25:59 AM UTC, original submission:

The following commit causes a fatal DOS condition in my setup when a client closes the connection:
http://git.savannah.gnu.org/cgit/lwip.git/commit/?id=236bc194222b604f251cd86d4715cd05d9e40f94

I can trigger the issue by having a client connect to the device (TCP server) and the device send unlimited amounts of data, then close the client application shortly. The result is that the client machine and the device get caught in an infinite loop, the device sending ACK and the client machine always responding with RST.

I am absolutely positive this commit is the cause. However I am using a significantly modified lwIP version.

Attached is a Wireshark capture of the event, and below is a text summary of the packets. The device is 192.168.111.142.

6 0.000663843 192.168.111.140 192.168.111.142 TCP 54 50210 → 80 [ACK] Seq=122206028 Ack=11397555 Win=64240 Len=0
7 0.000853303 192.168.111.142 192.168.111.140 TCP 254 80 → 50210 [PSH, ACK] Seq=11397555 Ack=122206028 Win=3341 Len=200
8 0.000872639 192.168.111.140 192.168.111.142 TCP 54 50210 → 80 [ACK] Seq=122206028 Ack=11397755 Win=64240 Len=0
9 0.001238707 192.168.111.140 192.168.111.142 TCP 54 50210 → 80 [FIN, ACK] Seq=122206028 Ack=11397755 Win=64240 Len=0
10 0.001277766 192.168.111.142 192.168.111.140 TCP 2974 80 → 50210 [PSH, ACK] Seq=11397755 Ack=122206028 Win=3341 Len=2920
11 0.001295877 192.168.111.140 192.168.111.142 TCP 54 50210 → 80 [RST] Seq=122206028 Win=0 Len=0
12 0.001378830 192.168.111.142 192.168.111.140 TCP 254 80 → 50210 [PSH, ACK] Seq=11400675 Ack=122206028 Win=3341 Len=200
13 0.001385864 192.168.111.140 192.168.111.142 TCP 54 50210 → 80 [RST] Seq=122206028 Win=0 Len=0
14 0.001387629 192.168.111.142 192.168.111.140 TCP 60 80 → 50210 [ACK] Seq=11400875 Ack=122206029 Win=3340 Len=0
15 0.001390811 192.168.111.140 192.168.111.142 TCP 54 50210 → 80 [RST] Seq=122206029 Win=0 Len=0
16 0.001548564 192.168.111.142 192.168.111.140 TCP 60 [TCP Dup ACK 14#1] 80 → 50210 [ACK] Seq=11400875 Ack=122206029 Win=3340 Len=0
17 0.001562509 192.168.111.140 192.168.111.142 TCP 54 50210 → 80 [RST] Seq=122206029 Win=0 Len=0
18 0.001667626 192.168.111.142 192.168.111.140 TCP 60 [TCP Dup ACK 14#2] 80 → 50210 [ACK] Seq=11400875 Ack=122206029 Win=3340 Len=0
19 0.001680022 192.168.111.140 192.168.111.142 TCP 54 50210 → 80 [RST] Seq=122206029 Win=0 Len=0
20 0.001771583 192.168.111.142 192.168.111.140 TCP 60 [TCP Dup ACK 14#3] 80 → 50210 [ACK] Seq=11400875 Ack=122206029 Win=3340 Len=0
21 0.001776081 192.168.111.140 192.168.111.142 TCP 54 50210 → 80 [RST] Seq=122206029 Win=0 Len=0

Ambroz Bizjak <abizjak>

 

Attached Files
file #37610:  error.pcapng added by abizjak (12KiB - application/x-pcapng)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by goldsimon (Updated the item)
  • -unavailable- added by gradator (Posted a comment)
  • -unavailable- added by axellin (Posted a comment)
  • -unavailable- added by abizjak (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 7 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Thu 30 Jun 2016 06:51:39 AM UTCgoldsimonStatusIn Progress=>Fixed
      Open/ClosedOpen=>Closed
    Thu 30 Jun 2016 06:27:00 AM UTCgoldsimonStatusNone=>In Progress
      Assigned toNone=>goldsimon
    Tue 28 Jun 2016 06:50:06 PM UTCgoldsimonPlanned Release=>2.0.0 Beta2
    Tue 28 Jun 2016 11:04:32 AM UTCaxellinAttached File-=>Added 0001-Fix-bug-48328-Fatal-RST-ACK-loop-caused-by-commit-fi.patch, #37611
    Tue 28 Jun 2016 10:26:00 AM UTCabizjakAttached File-=>Added error.pcapng, #37610

    Back to the top


    Powered by Savane 3.1-cleanup1