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: <CANn89iLSPdPvotnGhPb3Rq2gkmpn3kLGJO8=3PDFrhSjUQSAkg@mail.gmail.com>
Date: Fri, 17 Jan 2025 14:22:02 +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 12:30 PM Stephan Wurm <stephan.wurm@...berle.de> wrote:
>
> Hello Eric,
>
> Am 26. Nov 14:43 hat Eric Dumazet geschrieben:
> > syzbot is able to feed a packet with 14 bytes, pretending
> > it is a vlan one.
> >
> > Since fill_frame_info() is relying on skb->mac_len already,
> > extend the check to cover this case.
> thanks for addressing this szybot finding.
>
> Unfortunately, this seems to cause issues with VLAN tagged frames being
> dropped from a PRP interface.
>
> My setup consists of a custom embedded system equipped with v6.6 kernel,
> recently updated from v6.6.62 to v6.6.69. In order to gain support for
> VLAN tagged messages on top of PRP, I have applied first patch of the
> series (see msgid 20241106091710.3308519-2-danishanwar@...com) that is
> currently integrated with v6.13.
>
> Now I want to send GOOSE messages (L2 broadcast messages with VLAN
> header, including id=0 and QoS information) via the PRP interface.
> With v6.6.62 this works as expected, with v6.6.69 the functionality
> stopped again, with all VLAN-tagged frames being dropped from the PRP
> interface.
>
> By reverting this fix locally, I was able to restore the desired
> functionality. But I do not iyet understand, why this fix breaks
> sending of VLAN tagged frames in general.
>
> Do you already know about this side effect?
> Can you guide me to narrow down this issue?
>

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;
        }