netdev - Re: [PATCH net] net: hsr: avoid potential out-of-bound access in fill_frame

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <CANn89i+e-V4hkUmUALsJe3ZQYtTkxduN5Sv+OiV+vzEmOdU1+Q@mail.gmail.com>
Date: Fri, 17 Jan 2025 19:14:22 +0100
From: Eric Dumazet <edumazet@...gle.com>
To: Stephan Wurm <stephan.wurm@...berle.de>
Cc: "David S . Miller" <davem@...emloft.net>, Jakub Kicinski <kuba@...nel.org>, 
	Paolo Abeni <pabeni@...hat.com>, netdev@...r.kernel.org, eric.dumazet@...il.com, 
	syzbot+671e2853f9851d039551@...kaller.appspotmail.com, 
	WingMan Kwok <w-kwok2@...com>, Murali Karicheri <m-karicheri2@...com>, 
	MD Danish Anwar <danishanwar@...com>, Jiri Pirko <jiri@...dia.com>, 
	George McCollister <george.mccollister@...il.com>
Subject: Re: [PATCH net] net: hsr: avoid potential out-of-bound access in fill_frame_info()

On Fri, Jan 17, 2025 at 3:16 PM Stephan Wurm <stephan.wurm@...berle.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;
        }