netdev - Re: [PATCH net] net: hsr: fix fill_frame_info() regression vs VLAN packets

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+re0EQNt0K1=1XLsqApO=Qceu3Bix3ALWWFNd_E3KSnw@mail.gmail.com>
Date: Wed, 29 Jan 2025 16:32:15 +0100
From: Eric Dumazet <edumazet@...gle.com>
To: Simon Horman <horms@...nel.org>
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, 
	Stephan Wurm <stephan.wurm@...berle.de>
Subject: Re: [PATCH net] net: hsr: fix fill_frame_info() regression vs VLAN packets

On Wed, Jan 29, 2025 at 4:13 PM Simon Horman <horms@...nel.org> wrote:
>
> On Wed, Jan 29, 2025 at 01:00:07PM +0000, Eric Dumazet wrote:
> > Stephan Wurm reported that my recent patch broke VLAN support.
> >
> > Apparently skb->mac_len is not correct for VLAN traffic as
> > shown by debug traces [1].
> >
> > Use instead pskb_may_pull() to make sure the expected header
> > is present in skb->head.
> >
> > Many thanks to Stephan for his help.
> >
> > [1]
> > 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
> >
> > Fixes: b9653d19e556 ("net: hsr: avoid potential out-of-bound access in fill_frame_info()")
> > Reported-by: Stephan Wurm <stephan.wurm@...berle.de>
> > Tested-by: Stephan Wurm <stephan.wurm@...berle.de>
> > Closes: https://lore.kernel.org/netdev/Z4o_UC0HweBHJ_cw@PC-LX-SteWu/
> > Signed-off-by: Eric Dumazet <edumazet@...gle.com>
>
> Reviewed-by: Simon Horman <horms@...nel.org>
>
> Hi Eric,
>
> Just to clarify things for me:
>
> I see at the Closes link that you mention that
> "I am unsure why hsr_get_node() is working, since it also uses skb->mac_len".
>
> Did you gain any insight into that?
> If not, do you plan to look into it any further?

I have no plan to look into it.

Given HSR Orphan status, I guess that it does not matter much...