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

[Simon Horman]

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.

For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
---
net: hamradio: 6pack: fix uninit-value in sixpack_receive_buf

This commit fixes sixpack_receive_buf() to properly skip bytes with TTY
error flags by processing bytes one at a time and only passing non-error
bytes to sixpack_decode(). This matches the pattern used in slip_receive_buf()
and mkiss_receive_buf().

Closes: https://syzkaller.appspot.com/bug?extid=ecdb8c9878a81eb21e54

> diff --git a/drivers/net/hamradio/6pack.c b/drivers/net/hamradio/6pack.c
> --- a/drivers/net/hamradio/6pack.c
> +++ b/drivers/net/hamradio/6pack.c
> @@ -391,7 +391,6 @@ static void sixpack_receive_buf(struct tty_struct *tty, const u8 *cp,
>  				const u8 *fp, size_t count)
>  {
>  	struct sixpack *sp;
> -	size_t count1;
>
>  	if (!count)
>  		return;
> @@ -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);
>  }

The commit title states this fixes "uninit-value in sixpack_receive_buf", but
is this the exact cause of the KMSAN report?

The syzbot report confirms KMSAN detected uninit-value in sixpack_decode when
reading pre_rbuff[count1]. The uninit memory originated from tty_buffer_alloc
via kmalloc_flex (GFP_ATOMIC | __GFP_NOWARN, not zeroed).

However, bytes in the tty buffer should be initialized after being written by
serial8250_read_char -> uart_insert_char -> tty_insert_flip_char, which writes
both the character (from inb() I/O port read) and the flag. KMSAN's
__msan_instrument_asm_store() should unpoison inline asm outputs including
inb results.

The code fix addresses a genuine logic error where error-flagged bytes were
not skipped (claims 1-5 are verified true). But the connection between this
bug and the specific KMSAN uninit-value report is unclear, since the tty
bytes should be initialized after UART driver processing.

Could the KMSAN trigger involve subtle shadow tracking through tty buffer
allocation and inline asm instrumentation rather than the described code bug?
The syzbot report notes "Unfortunately, I don't have any reproducer for this
issue yet," which makes verification difficult.

The fix is correct regardless - it matches the established pattern in
slip_receive_buf() and mkiss_receive_buf() for handling error-flagged bytes.
Should the commit title more accurately describe what the fix addresses?