lwIP - A Lightweight TCP/IP stack - Bugs: bug #51447, Sequence number comparisons invoke...
You are not allowed to post comments on this tracker with your current authentication level.
bug #51447: Sequence number comparisons invoke implementation-defined behavior
Submitter: | Ambroz Bizjak <abizjak> | ||
Submitted: | Tue 11 Jul 2017 10:19:32 PM UTC | ||
Category: | TCP | Severity: | 3 - Normal |
Item Group: | Crash Error | Status: | In Progress |
Privacy: | Public | Assigned to: | goldsimon |
Open/Closed: | Open | Planned Release: | None |
lwIP version: | git head |
Jump to the original submission
Thu 31 Jan 2019 12:29:55 PM UTC, comment #15: |
Simon Goldschmidt <goldsimon> |
Thu 31 Jan 2019 09:52:02 AM UTC, comment #14: Response from Stian Skjelstad on mailinglist:
|
Dirk Ziegelmeier <dziegel> |
Tue 29 Jan 2019 12:20:07 PM UTC, comment #13: It's a bug, but it's not a bug in TCP_SEQ_BETWEEN.
|
Valery Ushakov <uwe> |
Tue 29 Jan 2019 09:49:20 AM UTC, comment #12: Reopen bug so this does not get lost |
Dirk Ziegelmeier <dziegel> |
Tue 29 Jan 2019 02:27:45 AM UTC, comment #11: I found some testcases caused a crash by this bug in fuzzing with AFL.
|
Hiromasa Ito <vhertz> |
Mon 17 Jul 2017 07:52:49 PM UTC, comment #10: @Simon, all can be expressed in terms of LT as I have shown in the first post. Here is passing test code showing my macros behave the same as current ones in edge cases (a=b-1, a=b, a=b+1).
|
Ambroz Bizjak <abizjak> |
Fri 14 Jul 2017 06:06:00 PM UTC, comment #9: Given that it works on all systems I know and this is purely theoretical, let's wait until we see the first quantum computers that have incompatible implementation-defined behaviour :-)
|
Simon Goldschmidt <goldsimon> |
Fri 14 Jul 2017 05:50:43 PM UTC, comment #8: For these cases, the subject can be changed after creation. |
Simon Goldschmidt <goldsimon> |
Thu 13 Jul 2017 10:54:02 PM UTC, comment #7: @uwe Yes I had already corrected myself, there is no need to reiterate it.
|
Ambroz Bizjak <abizjak> |
Thu 13 Jul 2017 09:54:17 PM UTC, comment #6: @abizjak - If you want to start nit-picking, you may at least try to be careful about your own wording. Your summary claims "undefined behavior", but then you quote the relevant part of the standard that says "implementation-defined behavior".
|
Valery Ushakov <uwe> |
Wed 12 Jul 2017 05:01:39 PM UTC, comment #5: See that GCC generates identical code for the s32 cast and my solution, showing these really are equivalent: https://godbolt.org/g/S4gp7J |
Ambroz Bizjak <abizjak> |
Wed 12 Jul 2017 04:59:56 PM UTC, comment #4: C99 standard says this:
|
Ambroz Bizjak <abizjak> |
Wed 12 Jul 2017 07:26:23 AM UTC, comment #3: ** Example: s16_t into u32_t will cause numbers to not be signed extended (-1 will become 0x0000ffff for example).
|
Stian Sebastian Skjelstad <mywave> |
Wed 12 Jul 2017 07:23:32 AM UTC, comment #2: typecasting between s32_t and u32_t is not undefined behaviour. On (almost all) systems, CPU work in 2nd compliments and the only differences about u32_t and s32_t are which branch instructions or which comparison instruction to use.
|
Stian Sebastian Skjelstad <mywave> |
Tue 11 Jul 2017 10:26:13 PM UTC, comment #1: forgot to cast the diferences to u32... the right code is:
|
Ambroz Bizjak <abizjak> |
Tue 11 Jul 2017 10:19:32 PM UTC, original submission:
See TCP_SEQ_LT and similar macros:
|
Ambroz Bizjak <abizjak> |
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 8 latest changes.
Date | Changed by | Updated Field | Previous Value | => | Replaced by |
---|---|---|---|---|---|
2019-01-31 | goldsimon | Status | None | In Progress | |
2019-01-29 | dziegel | Status | Wont Fix | None | |
Open/Closed | Closed | Open | |||
2017-07-14 | goldsimon | Status | None | Wont Fix | |
Assigned to | None | goldsimon | |||
Open/Closed | Open | Closed | |||
2017-07-14 | goldsimon | Summary | Sequence number comparisons invoke undefined behavior | Sequence number comparisons invoke implementation-defined behavior | |
2017-07-12 | mywave | Attached File | - | Added u32.c, #41185 |
Powered by Savane 3.13-d3ae.
Corresponding source code
It seems like this has been broken by commit a3d27e30e04bf1daa70dbe63e6c140f42fa7ab97 from 9th september 2004 which added TCP_SEQ_BETWEEN and uses it to replace various range tests.
That did work for valid segments, but it at least broke this check where 'seqno' and 'pcb->rcv_nxt' are exactly 0x80000000 apart.
I would fix it by reverting that commit for this line in question, but I first want to check the other changes that commit included to see if there are more problems of this kind.