drivers/slipdev: fix off-by-one error in _recv()#18229
drivers/slipdev: fix off-by-one error in _recv()#18229benpicco wants to merge 3 commits intoRIOT-OS:masterfrom
Conversation
If the number of written bytes is greater than the length of the buffer, we have already written out-of bounds memory. With pktbuf this means we will likely have corrupted the next free list entry.
benpicco
added
the
Type: bug
|
Agreeing with @kfessel. |
benpicco
force-pushed
the
drivers/slipdev-off-by-one
branch
from
06504fc to
f7bccf0
Compare
|
Like this? btw what's up with that |
|
Hm a still kills it |
drivers/slipdev/slipdev.c
Outdated
| if ((unsigned)res == len) { | ||
| /* clear out unreceived packet */ | ||
| while (byte != SLIPDEV_END) { | ||
| byte = tsrb_get_one(&dev->inbuf); | ||
| } | ||
| return -ENOBUFS; | ||
| } | ||
|
|
There was a problem hiding this comment.
| if ((unsigned)res == len) { | |
| /* clear out unreceived packet */ | |
| while (byte != SLIPDEV_END) { | |
| byte = tsrb_get_one(&dev->inbuf); | |
| } | |
| return -ENOBUFS; | |
| } | |
| if ( (unsigned) res >= len) { | |
| /* the result grew larger than the provided buffer | |
| clear out rest of the current packet, this package is lost */ | |
| do { | |
| byte = tsrb_get_one(&dev->inbuf); | |
| } while (byte != SLIPDEV_END); | |
| res = -ENOBUFS; | |
| break; | |
| } | |
There was a problem hiding this comment.
do{ } while to not depend on the initialization of byte
res = .. ; break; to avoid multiple returns
(unsigned) res >= len also catches negative res (for any reason) as bigger than len
and some comment cleanup (the old one sound like the package is unreceived (and might be still receivable)
There was a problem hiding this comment.
not sure if this helps with the ping issue but if not i don't think the original one helped either
|
Ah the adaptive ping issue is unrelated. I was testing this on a -> #17924 |
|
closed in favor of #18826 |
3 participants
Contribution description
If the number of written bytes is greater than the length of the buffer, we have already written out-of bounds memory.
With pktbuf this means we will likely have corrupted the next free list entry.
Testing procedure
Issues/PRs references
alternative to #18066