netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net] hsr: add length check before setting network header
@ 2025-08-20 18:03 Shigeru Yoshida
  2025-08-20 18:16 ` Eric Dumazet
  0 siblings, 1 reply; 3+ messages in thread
From: Shigeru Yoshida @ 2025-08-20 18:03 UTC (permalink / raw)
  To: davem, edumazet, kuba, pabeni, horms
  Cc: george.mccollister, netdev, linux-kernel, Shigeru Yoshida,
	syzbot+a81f2759d022496b40ab

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


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH net] hsr: add length check before setting network header
  2025-08-20 18:03 [PATCH net] hsr: add length check before setting network header Shigeru Yoshida
@ 2025-08-20 18:16 ` Eric Dumazet
  2025-08-21  1:08   ` Shigeru Yoshida
  0 siblings, 1 reply; 3+ messages in thread
From: Eric Dumazet @ 2025-08-20 18:16 UTC (permalink / raw)
  To: Shigeru Yoshida
  Cc: davem, kuba, pabeni, horms, george.mccollister, netdev,
	linux-kernel, syzbot+a81f2759d022496b40ab

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH net] hsr: add length check before setting network header
  2025-08-20 18:16 ` Eric Dumazet
@ 2025-08-21  1:08   ` Shigeru Yoshida
  0 siblings, 0 replies; 3+ messages in thread
From: Shigeru Yoshida @ 2025-08-21  1:08 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: davem, kuba, pabeni, horms, george.mccollister, netdev,
	linux-kernel, syzbot+a81f2759d022496b40ab

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


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2025-08-21  1:09 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-20 18:03 [PATCH net] hsr: add length check before setting network header Shigeru Yoshida
2025-08-20 18:16 ` Eric Dumazet
2025-08-21  1:08   ` Shigeru Yoshida

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).