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

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

[Mashiro Chen]

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@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=ecdb8c9878a81eb21e54
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Mashiro Chen <mashiro.chen@mailbox.org>
---
 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
@@ -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);
 }
-- 
2.53.0

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

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

[Mashiro Chen]

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@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=ecdb8c9878a81eb21e54
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Suggested-by: Simon Horman <horms@kernel.org>
Signed-off-by: Mashiro Chen <mashiro.chen@mailbox.org>
---
 drivers/net/hamradio/6pack.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/net/hamradio/6pack.c b/drivers/net/hamradio/6pack.c
index 885992951e8a6..9e5220ae98360 100644
--- 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,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);
 }
-- 
2.53.0

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?