Re: [PATCH net] net: hamradio: 6pack: fix uninit-value in sixpack_receive_buf
Simon Horman <[email protected]> Sat, 4 Apr 2026 09:56:58 +0100
| Newsgroups | org.kernel.vger.linux-hams,org.kernel.vger.linux-kernel,org.kernel.vger.netdev |
|---|---|
| Message-ID | <[email protected]> |
On Fri, Apr 03, 2026 at 12:45:25AM +0800, Mashiro Chen wrote: > sixpack_receive_buf() does not properly skip bytes with TTY error flags. > The while loop iterates through the flags buffer but never advances the > data pointer (cp), and passes the original count including error bytes > to sixpack_decode(). This causes sixpack_decode() to process bytes that > should have been skipped due to TTY errors. > > Fix this by processing bytes one at a time, advancing cp on each > iteration, and only passing non-error bytes to sixpack_decode(). > This matches the pattern used by slip_receive_buf() and > mkiss_receive_buf() for the same purpose. > > Reported-by: [email protected] > Closes: https://syzkaller.appspot.com/bug?extid=ecdb8c9878a81eb21e54 > Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") > Signed-off-by: Mashiro Chen <[email protected]> > --- > drivers/net/hamradio/6pack.c | 9 ++++----- > 1 file changed, 4 insertions(+), 5 deletions(-) > > diff --git a/drivers/net/hamradio/6pack.c b/drivers/net/hamradio/6pack.c > index 885992951e8a6..c8b2dc5c1becc 100644 > --- a/drivers/net/hamradio/6pack.c > +++ b/drivers/net/hamradio/6pack.c ... > @@ -401,16 +400,16 @@ static void sixpack_receive_buf(struct tty_struct *tty, const u8 *cp, > return; > > /* Read the characters out of the buffer */ > - count1 = count; > - while (count) { > - count--; > + while (count--) { > if (fp && *fp++) { > if (!test_and_set_bit(SIXPF_ERROR, &sp->flags)) > sp->dev->stats.rx_errors++; > + cp++; > continue; > } > + sixpack_decode(sp, cp, 1); > + cp++; > } > - sixpack_decode(sp, cp, count1); > > tty_unthrottle(tty); > } Hi, I am wondering if this could be expressed more succinctly by placing the cp++ in a common branch of execution. Something like this (completely untested!) @@ -401,16 +400,15 @@ static void sixpack_receive_buf(struct tty_struct *tty, const u8 *cp, return; /* Read the characters out of the buffer */ - count1 = count; - while (count) { - count--; + while (count--) { if (fp && *fp++) { if (!test_and_set_bit(SIXPF_ERROR, &sp->flags)) sp->dev->stats.rx_errors++; - continue; + } else { + sixpack_decode(sp, cp, 1); } + cp++; } - sixpack_decode(sp, cp, count1); tty_unthrottle(tty); } Also, while I don't think it is appropriate to be included in a fix. As a follow-up sixpack_decode() could be simplified by removing the count parameter, which is now always passed the value 1.