buglwIP - A Lightweight TCP/IP stack - Bugs: bug #36234, LISTEN send RST for incoming ACKs...

 
 

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

bug #36234: LISTEN send RST for incoming ACKs with seqno == ackno + 1

Submitted by:  Woody Wu <narke>
Submitted on:  Wed 18 Apr 2012 02:45:24 AM UTC  
Votes:  1  
 
Category: TCPSeverity: 3 - Normal
Item Group: NoneStatus: Works For Me
Privacy: PublicAssigned to: Simon Goldschmidt <goldsimon>
Open/Closed: ClosedPlanned Release: None
lwIP version: 1.4.0

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

Tue 02 Sep 2014 07:46:06 PM UTC, comment #11:

Can't reproduce (works for me!) and the OP didn't give any more insight...

Simon Goldschmidt <goldsimon>
Project AdministratorIn charge of this item.
Wed 28 May 2014 07:59:38 AM UTC, comment #10:

Our code is not calling directly tcp_abort().
It might be best that you close this bug based on the original post and I'll try to reproduce our issue (when I have the time...) with our latest code. If it happens again then I will create new bug with Wireshark log and LwIP trace prints etc.

Esa Hill <esah>
Tue 27 May 2014 08:06:17 PM UTC, comment #9:

From all I can test here and read from the code, a RST sent for an ACK to a LISTEN pcb correctly has seqno == ackno, so the original post is invalid to me. Please convince me of the opposite if I'm wrong.

Re comment #8:
Thank you for the information. However:

> I'm quoting same chapter from RCF as the original bug creator,
> RFC 793 page 35, state CLOSED

and comment #3:

> I think that the patch I included fixes this.


You're quoting the RFC for state CLOSED but the patch you provided changes code called for non-closed pcbs only. That makes no sense to me: tcp_abandon(), which you changed, is only called for connected PCBs (through tcp_abort()), or from tcp_listen_input on memory error (in this case, the response might indeed be wrong).

Unless your problem is tcp_listen_input having memory error (which it shouldn't be, since the incoming packet has no SYN set), how can changing tcp_abandon fix it? Did you call tcp_abort() yourself somewhere?

Simon Goldschmidt <goldsimon>
Project AdministratorIn charge of this item.
Fri 23 May 2014 07:44:15 AM UTC, comment #8:

I attached wireshark log and dug little deeper for the reason why we did this change.

In the log, packet 13 is marked by wireshark as 'TCP ACKed unseen segment' After that it takes about 17 seconds before Modbus connection is reopened. This long period might be PC's fault, but if seqno is correct, SYN is sent right away.

I'm quoting same chapter from RCF as the original bug creator, RFC 793 page 35, state CLOSED:

"1. If the connection does not exist (CLOSED) then a reset is sent..."

"If the incoming segment has an ACK field, the reset takes its
sequence number from the ACK field of the segment"

The packet 13 (RST ACK) has seqno=45, when the incoming packet 10 has ackno=44.

If I have understood RFC correctly, RST ACK should use the ackno from the received packet. Now it seems to use FIN ACK + 1 (which never left the device).

I can create new bug for our case if you want, but for me these look like same case. In both states it should use ackno from last segment.

(file #31421)

Esa Hill <esah>
Fri 23 May 2014 06:21:07 AM UTC, comment #7:

Alright, I got your description. But I still cannot see this from the RFC. Do you have a pcap log where this can be seen? If not, we'd have to reproduce this.

Anyway, your patch addresses something different to the summary of this bug, doesn't it? (This bug is about the 'tcp_rst' call in 'tcp_listen_input', where 'tcp_abandon' is not called)

Simon Goldschmidt <goldsimon>
Project AdministratorIn charge of this item.
Fri 23 May 2014 05:22:38 AM UTC, comment #6:

I had to find some old emails to recall this case and I still don't remember exact details, but for us this issue was fixed after changing that seqno.

I'm sure that SEQ=SND.NXT is correct but in our case FIN ACK with that seqno never left the device (cable unplugged). Then when cable was plugged back in and some packets for the old connection arrive, RST ACK is sent. The PC (win xp + third party SW) does not accept that RST ACK because seqno is +1 to what it expects (at least this was our conclusion). This then caused long delays before connection was reopened properly.

I guess this is a special case which could be fixed better, but this was the simplest change for our problem.

Esa Hill <esah>
Tue 20 May 2014 07:46:38 PM UTC, comment #5:

Re comment #3 in more detail:
Why do you think seqno==pcb->snd_nxt would be wrong? As I read RFC 793 (page 61), an ABORT call (which tcp_abandon is, effectively) should be sent with SEQ=SND.NXT, which is what I think we do.

We don't seem to check for SYN-SENT, CLOSING and LASTACK though, btu is this your case?

Simon Goldschmidt <goldsimon>
Project AdministratorIn charge of this item.
Tue 20 May 2014 07:40:31 PM UTC, comment #4:

Re orignal post:
I can't reproduce this. The packets I see are perfectly OK: RstSeq=InAck, RstAck=InSeq+1.
Given that, a pcap indeed would have been interesting.

Re comment #3:
Wouldn't that be a different bug? The summary of this bug says "LISTEN" state, whereas your bug seems to be related to connected pcbs?

Simon Goldschmidt <goldsimon>
Project AdministratorIn charge of this item.
Wed 03 Apr 2013 12:28:59 PM UTC, comment #3:

We had similar issue where RST ACK had sequency number +1. I think that the patch I included fixes this.

The patch is to STABLE-1_4_1

Details are here: http://lists.nongnu.org/archive/html/lwip-users/2013-03/msg00037.html

Change was done to tcp.c:375
seqno = pcb->snd_nxt; => seqno = pcb->lastack;

(file #27754)

Esa Hill <esah>
Tue 19 Jun 2012 07:07:18 PM UTC, comment #2:

I don't think an pcap is necessary - it's probably just a straightforward bug in lwIP.

Kieran Mansley <kieranm>
Project Administrator
Fri 08 Jun 2012 08:03:18 AM UTC, comment #1:

Please upload a pcap file, we can know more about it.

tao tang <opensms4>
Wed 18 Apr 2012 02:45:24 AM UTC, original submission:

Hi,

For the reset generation, page 35 of RFC 793 says,

If the incoming segment has an ACK field, the reset takes its
sequence number from the ACK field of the segment, otherwise the
reset has sequence number zero and the ACK field is set to the sum
of the sequence number and segment length of the incoming segment.

But according to my test, when pcb is in LISTEN state and received a ACK, the reset segment it sent out has a seqno of ackno + 1 of the incoming ACK.

Woody Wu <narke>

 

Attached Files
file #31421:  seqno_plus_1.pcap added by esah (2KiB - application/octet-stream)
file #27754:  rst_ack.patch added by esah (629B - application/octet-stream - Fix proposal)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by goldsimon (Posted a comment)
  • -unavailable- added by esah (Updated the item)
  • -unavailable- added by esah (Voted in favor of this item)
  • -unavailable- added by kieranm (Posted a comment)
  • -unavailable- added by opensms4 (Posted a comment)
  • -unavailable- added by narke (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 1 encouragement so far.

    Only logged-in users can vote.

     

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

     

     

    Follow 8 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Tue 02 Sep 2014 07:46:06 PM UTCgoldsimonStatusNeed Info=>Works For Me
      Assigned toNone=>goldsimon
      Open/ClosedOpen=>Closed
    Fri 23 May 2014 07:44:14 AM UTCesahAttached File-=>Added seqno_plus_1.pcap, #31421
    Tue 20 May 2014 07:46:38 PM UTCgoldsimonStatusWorks For Me=>Need Info
    Tue 20 May 2014 07:40:31 PM UTCgoldsimonStatusNone=>Works For Me
    Wed 03 Apr 2013 12:28:59 PM UTCesahAttached File-=>Added rst_ack.patch, #27754
      Carbon-Copy-=>Added esah

    Back to the top


    Powered by Savane 3.1-cleanup1