netdev - Re: [PATCH net] hsr: add length check before setting network header

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CANn89iJQsEXYR9wWoztv1SnoQcaRxKyyx7X7j_VDfvdJi4cfhw@mail.gmail.com>
Date: Wed, 20 Aug 2025 11:16:21 -0700
From: Eric Dumazet <edumazet@...gle.com>
To: Shigeru Yoshida <syoshida@...hat.com>
Cc: davem@...emloft.net, kuba@...nel.org, pabeni@...hat.com, horms@...nel.org, 
	george.mccollister@...il.com, netdev@...r.kernel.org, 
	linux-kernel@...r.kernel.org, 
	syzbot+a81f2759d022496b40ab@...kaller.appspotmail.com
Subject: Re: [PATCH net] hsr: add length check before setting network header

On Wed, Aug 20, 2025 at 11:04 AM Shigeru Yoshida <syoshida@...hat.com> wrote:
>
> syzbot reported an uninitialized value issue in hsr_get_node() [1].
> If the packet length is insufficient, it can lead to the issue when
> accessing HSR header.
>
> Add validation to ensure sufficient packet length before setting
> network header in HSR frame handling to prevent the issue.
>
> [1]
> BUG: KMSAN: uninit-value in hsr_get_node+0xab0/0xad0 net/hsr/hsr_framereg.c:250
>  hsr_get_node+0xab0/0xad0 net/hsr/hsr_framereg.c:250
>  fill_frame_info net/hsr/hsr_forward.c:577 [inline]
>  hsr_forward_skb+0x330/0x30e0 net/hsr/hsr_forward.c:615
>  hsr_handle_frame+0xa20/0xb50 net/hsr/hsr_slave.c:69
>  __netif_receive_skb_core+0x1cff/0x6190 net/core/dev.c:5432
>  __netif_receive_skb_one_core net/core/dev.c:5536 [inline]
>  __netif_receive_skb+0xca/0xa00 net/core/dev.c:5652
>  netif_receive_skb_internal net/core/dev.c:5738 [inline]
>  netif_receive_skb+0x58/0x660 net/core/dev.c:5798
>  tun_rx_batched+0x3ee/0x980 drivers/net/tun.c:1549
>  tun_get_user+0x5566/0x69e0 drivers/net/tun.c:2002
>  tun_chr_write_iter+0x3af/0x5d0 drivers/net/tun.c:2048
>  call_write_iter include/linux/fs.h:2110 [inline]
>  new_sync_write fs/read_write.c:497 [inline]
>  vfs_write+0xb63/0x1520 fs/read_write.c:590
>  ksys_write+0x20f/0x4c0 fs/read_write.c:643
>  __do_sys_write fs/read_write.c:655 [inline]
>  __se_sys_write fs/read_write.c:652 [inline]
>  __x64_sys_write+0x93/0xe0 fs/read_write.c:652
>  x64_sys_call+0x3062/0x3b50 arch/x86/include/generated/asm/syscalls_64.h:2
>  do_syscall_x64 arch/x86/entry/common.c:52 [inline]
>  do_syscall_64+0xcf/0x1e0 arch/x86/entry/common.c:83
>  entry_SYSCALL_64_after_hwframe+0x77/0x7f
>
> Uninit was created at:
>  __alloc_pages+0x9d6/0xe70 mm/page_alloc.c:4598
>  alloc_pages_mpol+0x299/0x990 mm/mempolicy.c:2264
>  alloc_pages+0x1bf/0x1e0 mm/mempolicy.c:2335
>  skb_page_frag_refill+0x2bf/0x7c0 net/core/sock.c:2921
>  tun_build_skb drivers/net/tun.c:1679 [inline]
>  tun_get_user+0x1258/0x69e0 drivers/net/tun.c:1819
>  tun_chr_write_iter+0x3af/0x5d0 drivers/net/tun.c:2048
>  call_write_iter include/linux/fs.h:2110 [inline]
>  new_sync_write fs/read_write.c:497 [inline]
>  vfs_write+0xb63/0x1520 fs/read_write.c:590
>  ksys_write+0x20f/0x4c0 fs/read_write.c:643
>  __do_sys_write fs/read_write.c:655 [inline]
>  __se_sys_write fs/read_write.c:652 [inline]
>  __x64_sys_write+0x93/0xe0 fs/read_write.c:652
>  x64_sys_call+0x3062/0x3b50 arch/x86/include/generated/asm/syscalls_64.h:2
>  do_syscall_x64 arch/x86/entry/common.c:52 [inline]
>  do_syscall_64+0xcf/0x1e0 arch/x86/entry/common.c:83
>  entry_SYSCALL_64_after_hwframe+0x77/0x7f
>
> CPU: 1 PID: 5050 Comm: syz-executor387 Not tainted 6.9.0-rc4-syzkaller #0
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 03/27/2024
>
> Fixes: 48b491a5cc74 ("net: hsr: fix mac_len checks")
> Reported-by: syzbot+a81f2759d022496b40ab@...kaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=a81f2759d022496b40ab
> Tested-by: syzbot+a81f2759d022496b40ab@...kaller.appspotmail.com
> Signed-off-by: Shigeru Yoshida <syoshida@...hat.com>
> ---
>  net/hsr/hsr_slave.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/net/hsr/hsr_slave.c b/net/hsr/hsr_slave.c
> index b87b6a6fe070..979fe4084f86 100644
> --- a/net/hsr/hsr_slave.c
> +++ b/net/hsr/hsr_slave.c
> @@ -63,8 +63,12 @@ static rx_handler_result_t hsr_handle_frame(struct sk_buff **pskb)
>         skb_push(skb, ETH_HLEN);
>         skb_reset_mac_header(skb);
>         if ((!hsr->prot_version && protocol == htons(ETH_P_PRP)) ||
> -           protocol == htons(ETH_P_HSR))
> +           protocol == htons(ETH_P_HSR)) {
> +               if (skb->len < ETH_HLEN + HSR_HLEN)
> +                       goto finish_pass;
> +
>                 skb_set_network_header(skb, ETH_HLEN + HSR_HLEN);
> +       }
>         skb_reset_mac_len(skb);
>
>         /* Only the frames received over the interlink port will assign a
> --
> 2.50.1
>

You probably have missed a more correct fix :

https://www.spinics.net/lists/netdev/msg1116106.html