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

[Shigeru Yoshida <syoshida@redhat.com>]

On Wed, Aug 20, 2025 at 11:16:21AM -0700, Eric Dumazet wrote:
> On Wed, Aug 20, 2025 at 11:04 AM Shigeru Yoshida <syoshida@redhat.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@syzkaller.appspotmail.com
> > Closes: https://syzkaller.appspot.com/bug?extid=a81f2759d022496b40ab
> > Tested-by: syzbot+a81f2759d022496b40ab@syzkaller.appspotmail.com
> > Signed-off-by: Shigeru Yoshida <syoshida@redhat.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

Hi Eric,

Yes, I missed the patch you mentioned. Sorry~

Shigeru