From: Stephan Wurm <stephan.wurm@a-eberle.de>
To: Eric Dumazet <edumazet@google.com>
Cc: "David S . Miller" <davem@davemloft.net>,
Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
netdev@vger.kernel.org, eric.dumazet@gmail.com,
syzbot+671e2853f9851d039551@syzkaller.appspotmail.com,
WingMan Kwok <w-kwok2@ti.com>,
Murali Karicheri <m-karicheri2@ti.com>,
MD Danish Anwar <danishanwar@ti.com>,
Jiri Pirko <jiri@nvidia.com>,
George McCollister <george.mccollister@gmail.com>
Subject: Re: [PATCH net] net: hsr: avoid potential out-of-bound access in fill_frame_info()
Date: Mon, 20 Jan 2025 08:31:29 +0100 [thread overview]
Message-ID: <Z4370QW5kLDptEEQ@PC-LX-SteWu> (raw)
In-Reply-To: <CANn89iJghv1JSwO7AVh97mU1Laj11SooiioZOHJ+UbUVeAcKUQ@mail.gmail.com>
Am 17. Jan 19:18 hat Eric Dumazet geschrieben:
> On Fri, Jan 17, 2025 at 7:14 PM Eric Dumazet <edumazet@google.com> wrote:
> >
> > On Fri, Jan 17, 2025 at 3:16 PM Stephan Wurm <stephan.wurm@a-eberle.de> wrote:
> > >
> > > Am 17. Jan 14:22 hat Eric Dumazet geschrieben:
> > > >
> > > > Thanks for the report !
> > > >
> > > > You could add instrumentation there so that we see packet content.
> > > >
> > > > I suspect mac_len was not properly set somewhere.
> > > >
> > > > diff --git a/net/hsr/hsr_forward.c b/net/hsr/hsr_forward.c
> > > > index 87bb3a91598ee96b825f7aaff53aafb32ffe4f95..b0068e23083416ba13794e3b152517afbe5125b7
> > > > 100644
> > > > --- a/net/hsr/hsr_forward.c
> > > > +++ b/net/hsr/hsr_forward.c
> > > > @@ -700,8 +700,10 @@ static int fill_frame_info(struct hsr_frame_info *frame,
> > > > frame->is_vlan = true;
> > > >
> > > > if (frame->is_vlan) {
> > > > - if (skb->mac_len < offsetofend(struct hsr_vlan_ethhdr, vlanhdr))
> > > > + if (skb->mac_len < offsetofend(struct hsr_vlan_ethhdr,
> > > > vlanhdr)) {
> > > > + DO_ONCE_LITE(skb_dump, KERN_ERR, skb, true);
> > > > return -EINVAL;
> > > > + }
> > > > vlan_hdr = (struct hsr_vlan_ethhdr *)ethhdr;
> > > > proto = vlan_hdr->vlanhdr.h_vlan_encapsulated_proto;
> > > > }
> > >
> > > Thanks for your instrumentation patch.
> > >
> > > I got the following output in kernel log when sending an icmp echo with
> > > VLAN header:
> > >
> > > kernel: prp0: entered promiscuous mode
> > > kernel: skb len=46 headroom=2 headlen=46 tailroom=144
> > > mac=(2,14) net=(16,-1) trans=-1
> > > shinfo(txflags=0 nr_frags=0 gso(size=0 type=0 segs=0))
> > > csum(0x0 ip_summed=0 complete_sw=0 valid=0 level=0)
> > > hash(0x0 sw=0 l4=0) proto=0x0000 pkttype=0 iif=0
> > > kernel: dev name=prp0 feat=0x0000000000007000
> > > kernel: sk family=17 type=3 proto=0
> > > kernel: skb headroom: 00000000: 0d 12
> > > kernel: skb linear: 00000000: 00 d0 93 4a 2d 91 00 d0 93 53 9c cb 81 00 00 00
> > > kernel: skb linear: 00000010: 08 00 45 00 00 1c 00 01 00 00 40 01 d4 a1 ac 10
> > > kernel: skb linear: 00000020: 27 14 ac 10 27 0a 08 00 f7 ff 00 00 00 00
> > > kernel: skb tailroom: 00000000: 00 01 00 06 20 03 00 25 3c 20 00 00 00 00 00 00
> > > kernel: skb tailroom: 00000010: 00 00 00 00 00 00 00 00 00 00 00 00 00 01 00 3d
> > > kernel: skb tailroom: 00000020: 00 00 00 00 67 8a 61 45 15 63 56 39 00 25 00 7f
> > > kernel: skb tailroom: 00000030: f8 fe ff ff 7f 00 d0 93 ff fe 64 e8 8e 00 53 00
> > > kernel: skb tailroom: 00000040: 14 0e 14 31 00 00 53 00 14 0e 14 29 00 00 00 00
> > > kernel: skb tailroom: 00000050: 00 00 00 00 00 00 00 00 00 00 08 00 45 00 00 34
> > > kernel: skb tailroom: 00000060: 24 fa 40 00 40 06 17 c8 7f 00 00 01 7f 00 00 01
> > > kernel: skb tailroom: 00000070: aa 04 13 8c 94 1d a0 b2 77 d6 5f 8a 80 10 02 00
> > > kernel: skb tailroom: 00000080: fe 28 00 00 01 01 08 0a 89 e9 8a f7 89 e9 8a f7
> > > kernel: prp0: left promiscuous mode
> > >
> >
> > Yup, mac_len is incorrect, and the network header is also wrong.
> >
> > Please give us a stack trace, because at least one caller of
> > hsr_forward() needs to be VLAN ready.
> >
> > diff --git a/net/hsr/hsr_forward.c b/net/hsr/hsr_forward.c
> > index 87bb3a91598ee96b825f7aaff53aafb32ffe4f95..6f65a535c7fcd740cef81e718323e86fd1eef832
> > 100644
> > --- a/net/hsr/hsr_forward.c
> > +++ b/net/hsr/hsr_forward.c
> > @@ -700,8 +700,11 @@ static int fill_frame_info(struct hsr_frame_info *frame,
> > frame->is_vlan = true;
> >
> > if (frame->is_vlan) {
> > - if (skb->mac_len < offsetofend(struct hsr_vlan_ethhdr, vlanhdr))
> > + if (skb->mac_len < offsetofend(struct hsr_vlan_ethhdr,
> > vlanhdr)) {
> > + DO_ONCE_LITE(skb_dump, KERN_ERR, skb, true);
> > + WARN_ON_ONCE(1);
> > return -EINVAL;
> > + }
> > vlan_hdr = (struct hsr_vlan_ethhdr *)ethhdr;
> > proto = vlan_hdr->vlanhdr.h_vlan_encapsulated_proto;
> > }
>
> BTW, also please cherry-pick this commit from linux-6.10
>
> commit 4308811ba90118ae1b71a95fee79ab7dada6400c
> Author: Eric Dumazet <edumazet@google.com>
> Date: Sun Apr 7 08:06:06 2024 +0000
>
> net: display more skb fields in skb_dump()
Applying the new instrumentation gives me the following stack trace:
kernel: skb len=170 headroom=2 headlen=170 tailroom=20
mac=(2,14) mac_len=14 net=(16,-1) trans=-1
shinfo(txflags=0 nr_frags=0 gso(size=0 type=0 segs=0))
csum(0x0 start=0 offset=0 ip_summed=0 complete_sw=0 valid=0 level=0)
hash(0x0 sw=0 l4=0) proto=0x0000 pkttype=0 iif=0
priority=0x0 mark=0x0 alloc_cpu=0 vlan_all=0x0
encapsulation=0 inner(proto=0x0000, mac=0, net=0, trans=0)
kernel: dev name=prp0 feat=0x0000000000007000
kernel: sk family=17 type=3 proto=0
kernel: skb headroom: 00000000: 74 00
kernel: skb linear: 00000000: 01 0c cd 01 00 01 00 d0 93 53 9c cb 81 00 80 00
kernel: skb linear: 00000010: 88 b8 00 01 00 98 00 00 00 00 61 81 8d 80 16 52
kernel: skb linear: 00000020: 45 47 44 4e 43 54 52 4c 2f 4c 4c 4e 30 24 47 4f
kernel: skb linear: 00000030: 24 47 6f 43 62 81 01 14 82 16 52 45 47 44 4e 43
kernel: skb linear: 00000040: 54 52 4c 2f 4c 4c 4e 30 24 44 73 47 6f 6f 73 65
kernel: skb linear: 00000050: 83 07 47 6f 49 64 65 6e 74 84 08 67 8d f5 93 7e
kernel: skb linear: 00000060: 76 c8 00 85 01 01 86 01 00 87 01 00 88 01 01 89
kernel: skb linear: 00000070: 01 00 8a 01 02 ab 33 a2 15 83 01 00 84 03 03 00
kernel: skb linear: 00000080: 00 91 08 67 8d f5 92 77 4b c6 1f 83 01 00 a2 1a
kernel: skb linear: 00000090: a2 06 85 01 00 83 01 00 84 03 03 00 00 91 08 67
kernel: skb linear: 000000a0: 8d f5 92 77 4b c6 1f 83 01 00
kernel: skb tailroom: 00000000: 80 18 02 00 fe 4e 00 00 01 01 08 0a 4f fd 5e d1
kernel: skb tailroom: 00000010: 4f fd 5e cd
kernel: ------------[ cut here ]------------
kernel: WARNING: CPU: 0 PID: 751 at /net/hsr/hsr_forward.c:605 fill_frame_info+0x180/0x19c
kernel: Modules linked in:
kernel: CPU: 0 PID: 751 Comm: reg61850 Not tainted 6.6.69-ga7a5cc0c39f0 #1
kernel: Hardware name: Freescale LS1021A
kernel: unwind_backtrace from show_stack+0x10/0x14
kernel: show_stack from dump_stack_lvl+0x40/0x4c
kernel: dump_stack_lvl from __warn+0x94/0xc0
kernel: __warn from warn_slowpath_fmt+0x1b4/0x1bc
kernel: warn_slowpath_fmt from fill_frame_info+0x180/0x19c
kernel: fill_frame_info from hsr_forward_skb+0x54/0x118
kernel: hsr_forward_skb from hsr_dev_xmit+0x60/0xc4
kernel: hsr_dev_xmit from dev_hard_start_xmit+0xa0/0xe4
kernel: dev_hard_start_xmit from __dev_queue_xmit+0x144/0x5e8
kernel: __dev_queue_xmit from packet_snd+0x5c0/0x784
kernel: packet_snd from sock_write_iter+0xa0/0x10c
kernel: sock_write_iter from vfs_write+0x3ac/0x41c
kernel: vfs_write from ksys_write+0xbc/0xf0
kernel: ksys_write from ret_fast_syscall+0x0/0x4c
kernel: Exception stack(0xc0d8dfa8 to 0xc0d8dff0)
kernel: dfa0: 000000aa 73058e53 00000012 73058e53 000000aa 00000000
kernel: dfc0: 000000aa 73058e53 00000012 00000004 6ebf9940 0000000a 00000000 00000000
kernel: dfe0: 00000004 6ebf90f8 766a17ad 7661e5e6
kernel: ---[ end trace 0000000000000000 ]---
next prev parent reply other threads:[~2025-01-20 7:33 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-11-26 14:43 [PATCH net] net: hsr: avoid potential out-of-bound access in fill_frame_info() Eric Dumazet
2024-11-30 22:20 ` patchwork-bot+netdevbpf
2025-01-17 11:30 ` Stephan Wurm
2025-01-17 13:22 ` Eric Dumazet
2025-01-17 14:15 ` Stephan Wurm
2025-01-17 18:14 ` Eric Dumazet
2025-01-17 18:18 ` Eric Dumazet
2025-01-20 7:31 ` Stephan Wurm [this message]
2025-01-20 12:24 ` Eric Dumazet
2025-01-21 15:14 ` Stephan Wurm
2025-01-21 15:35 ` Eric Dumazet
2025-01-22 10:26 ` Stephan Wurm
2025-01-22 10:29 ` Eric Dumazet
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=Z4370QW5kLDptEEQ@PC-LX-SteWu \
--to=stephan.wurm@a-eberle.de \
--cc=danishanwar@ti.com \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=eric.dumazet@gmail.com \
--cc=george.mccollister@gmail.com \
--cc=jiri@nvidia.com \
--cc=kuba@kernel.org \
--cc=m-karicheri2@ti.com \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
--cc=syzbot+671e2853f9851d039551@syzkaller.appspotmail.com \
--cc=w-kwok2@ti.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox