Re: [PATCH net] net: hamradio: 6pack: fix uninit-value in sixpack_receive_buf

[Simon Horman]

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: syzbot+ecdb8c9878a81eb21e54@xxxxxxxxxxxxxxxxxxxxxxxxx
> Closes: https://syzkaller.appspot.com/bug?extid=ecdb8c9878a81eb21e54
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Signed-off-by: Mashiro Chen <mashiro.chen@xxxxxxxxxxx>
> ---
>  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.