lwIP - A Lightweight TCP/IP stack - Bugs: bug #28288, Data after FIN in oos queue
You are not allowed to post comments on this tracker with your current authentication level.
bug #28288: Data after FIN in oos queue
Submitter: | Oleg Tyshev <olegreen> | ||
Submitted: | Mon 14 Dec 2009 10:59:34 AM UTC | ||
Category: | TCP | Severity: | 3 - Normal |
Item Group: | Faulty Behaviour | Status: | Fixed |
Privacy: | Public | Assigned to: | goldsimon |
Open/Closed: | Closed | Planned Release: | None |
lwIP version: | CVS Head |
Jump to the original submission
Tue 27 Sep 2011 07:17:52 PM UTC, comment #16: |
Simon Goldschmidt <goldsimon> |
Tue 27 Sep 2011 01:36:19 PM UTC, comment #15: So that would mean ((state >= ESTABLISHED) && (state < CLOSE_WAIT))? That would be OK to add, but it would not change anything since tcp_receive() is no called for states < ESTABLISHED. Adding an assert (plus a comment) would be the best thing to do to make the code clearer, I guess. |
Simon Goldschmidt <goldsimon> |
Tue 27 Sep 2011 12:59:50 PM UTC, comment #14: Looks like an improvement, although I'm concerned about accepting data in other invalid states that are < CLOSE_WAIT. Really we should only accept data in ESTABLISHED or one of the two FIN_WAIT states. |
Kieran Mansley <kieranm> |
Mon 26 Sep 2011 08:14:14 PM UTC, comment #13: After writing unit tests for this, it turns out that the only thing lwIP missed is to reject data or FIN after the first FIN has been received (the ooseq code correctly dismisses all data after an ooseq FIN).
|
Simon Goldschmidt <goldsimon> |
Tue 04 May 2010 07:38:03 PM UTC, comment #12: I think this can be delayed, too, since it doesn't involve the API at all, so won't result in API changes. |
Simon Goldschmidt <goldsimon> |
Tue 16 Feb 2010 06:15:31 PM UTC, comment #11: Even case 1 is possibly solved best by keeping the seqno of a FIN (once received, no matter if inseq or ooseq) in the pcb. We can then check any incoming segment:
|
Simon Goldschmidt <goldsimon> |
Tue 12 Jan 2010 03:58:23 PM UTC, comment #10: For 3 (received FIN is inseq, another inseq FIN has been received before) this is just a generalisation of receiving any data after an earlier FIN. We could leave rcv_nxt for the pcb at the sequence following the FIN to store that value. I would expect the RFC behaviour here to be to drop the data after the FIN and reset the connection to indicate that some data have been dropped, but I haven't checked. |
Kieran Mansley <kieranm> |
Mon 11 Jan 2010 05:49:21 PM UTC, comment #9: Possible states for duplicate FINs are
|
Simon Goldschmidt <goldsimon> |
Tue 15 Dec 2009 01:26:32 PM UTC, comment #8: The only think I don't like in data after FIN
|
Oleg Tyshev <olegreen> |
Tue 15 Dec 2009 09:25:28 AM UTC, comment #7: limit on data on ooseq queue: sounds like a good feature for 1.4.0
|
Kieran Mansley <kieranm> |
Tue 15 Dec 2009 08:07:56 AM UTC, comment #6: I don't know if I would see that as an exploit: as Oleg said, it might lead to the application seeing more data than it should, but nothing more.
|
Simon Goldschmidt <goldsimon> |
Tue 15 Dec 2009 07:56:13 AM UTC, comment #5: OK, this would be a peer's bug. But would it be an exploit in lwip? |
Iordan Neshev <iordan_neshev> |
Mon 14 Dec 2009 04:19:33 PM UTC, comment #4:
|
Simon Goldschmidt <goldsimon> |
Mon 14 Dec 2009 03:04:55 PM UTC, comment #3: If we get two FINs at different sequence numbers we should definitely reset the connection so that the application at the other end might be made aware that there has been a problem and some data are not delivered. I agree this is low priority, but is definitely something we could improve our handling, even if it is the other end at fault.
|
Kieran Mansley <kieranm> |
Mon 14 Dec 2009 12:28:18 PM UTC, comment #2: In current implementation as I can see data after FIN from oos queue will be transfered to the application too if (pcb->ooseq->tcphdr->seqno == pcb->rcv_nxt).
|
Oleg Tyshev <olegreen> |
Mon 14 Dec 2009 11:56:12 AM UTC, comment #1: First of all, correct me if I'm wrong, but I guess the only impact will be wasted memory: once the segments are dequeued, they will be removed after FIN is processed.
|
Simon Goldschmidt <goldsimon> |
Mon 14 Dec 2009 10:59:34 AM UTC, original submission:
Kieran in another thread (#28241) noticed that data after FIN doesn't have special processing.
|
Oleg Tyshev <olegreen> |
No files currently attached
Depends on the following items: None found
Items that depend on this one: None found
There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.
Follow 7 latest changes.
Date | Changed by | Updated Field | Previous Value | => | Replaced by |
---|---|---|---|---|---|
2011-09-27 | goldsimon | Status | Need Info | Fixed | |
Open/Closed | Open | Closed | |||
2011-09-26 | goldsimon | Status | None | Need Info | |
Assigned to | None | goldsimon | |||
2010-05-18 | kieranm | Planned Release | 1.4.x | 1.4.1 | |
2010-05-04 | goldsimon | Planned Release | 1.4.0 | 1.4.x | |
2010-04-21 | goldsimon | Planned Release | 1.4.0 |
Powered by Savane 3.13-02a9.
Corresponding source code
I've added the check (for state < CLOSE_WAIT) and the assert (for state >= ESTABLISHE), so that tcp_receive is only called for processing ACK and data in ESTABLISHED/FIN_WAIT_1/2 and called for processing ACKs in CLOSE_WAIT, CLOSING, LAST_ACK and TIME_WAIT.
Closing as fixed (as my unit test show), please reopen if you think differently.