* [PATCH net] net: hsr: fix fill_frame_info() regression vs VLAN packets
@ 2025-01-29 13:00 Eric Dumazet
2025-01-29 15:13 ` Simon Horman
2025-01-30 17:10 ` patchwork-bot+netdevbpf
0 siblings, 2 replies; 4+ messages in thread
From: Eric Dumazet @ 2025-01-29 13:00 UTC (permalink / raw)
To: David S . Miller, Jakub Kicinski, Paolo Abeni
Cc: netdev, Simon Horman, eric.dumazet, Eric Dumazet, Stephan Wurm
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@a-eberle.de>
Tested-by: Stephan Wurm <stephan.wurm@a-eberle.de>
Closes: https://lore.kernel.org/netdev/Z4o_UC0HweBHJ_cw@PC-LX-SteWu/
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
net/hsr/hsr_forward.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/net/hsr/hsr_forward.c b/net/hsr/hsr_forward.c
index 87bb3a91598ee96b825f7aaff53aafb32ffe4f9..c80575db8b91d9bcce376e1a8c3fda76aa91c17 100644
--- a/net/hsr/hsr_forward.c
+++ b/net/hsr/hsr_forward.c
@@ -700,9 +700,12 @@ 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))
+ /* Note: skb->mac_len might be wrong here. */
+ if (!pskb_may_pull(skb,
+ skb_mac_offset(skb) +
+ offsetofend(struct hsr_vlan_ethhdr, vlanhdr)))
return -EINVAL;
- vlan_hdr = (struct hsr_vlan_ethhdr *)ethhdr;
+ vlan_hdr = (struct hsr_vlan_ethhdr *)skb_mac_header(skb);
proto = vlan_hdr->vlanhdr.h_vlan_encapsulated_proto;
}
--
2.48.1.262.g85cc9f2d1e-goog
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH net] net: hsr: fix fill_frame_info() regression vs VLAN packets
2025-01-29 13:00 [PATCH net] net: hsr: fix fill_frame_info() regression vs VLAN packets Eric Dumazet
@ 2025-01-29 15:13 ` Simon Horman
2025-01-29 15:32 ` Eric Dumazet
2025-01-30 17:10 ` patchwork-bot+netdevbpf
1 sibling, 1 reply; 4+ messages in thread
From: Simon Horman @ 2025-01-29 15:13 UTC (permalink / raw)
To: Eric Dumazet
Cc: David S. Miller, Jakub Kicinski, Paolo Abeni, netdev,
eric.dumazet, Stephan Wurm
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@a-eberle.de>
> Tested-by: Stephan Wurm <stephan.wurm@a-eberle.de>
> Closes: https://lore.kernel.org/netdev/Z4o_UC0HweBHJ_cw@PC-LX-SteWu/
> Signed-off-by: Eric Dumazet <edumazet@google.com>
Reviewed-by: Simon Horman <horms@kernel.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?
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH net] net: hsr: fix fill_frame_info() regression vs VLAN packets
2025-01-29 15:13 ` Simon Horman
@ 2025-01-29 15:32 ` Eric Dumazet
0 siblings, 0 replies; 4+ messages in thread
From: Eric Dumazet @ 2025-01-29 15:32 UTC (permalink / raw)
To: Simon Horman
Cc: David S. Miller, Jakub Kicinski, Paolo Abeni, netdev,
eric.dumazet, Stephan Wurm
On Wed, Jan 29, 2025 at 4:13 PM Simon Horman <horms@kernel.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@a-eberle.de>
> > Tested-by: Stephan Wurm <stephan.wurm@a-eberle.de>
> > Closes: https://lore.kernel.org/netdev/Z4o_UC0HweBHJ_cw@PC-LX-SteWu/
> > Signed-off-by: Eric Dumazet <edumazet@google.com>
>
> Reviewed-by: Simon Horman <horms@kernel.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...
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH net] net: hsr: fix fill_frame_info() regression vs VLAN packets
2025-01-29 13:00 [PATCH net] net: hsr: fix fill_frame_info() regression vs VLAN packets Eric Dumazet
2025-01-29 15:13 ` Simon Horman
@ 2025-01-30 17:10 ` patchwork-bot+netdevbpf
1 sibling, 0 replies; 4+ messages in thread
From: patchwork-bot+netdevbpf @ 2025-01-30 17:10 UTC (permalink / raw)
To: Eric Dumazet
Cc: davem, kuba, pabeni, netdev, horms, eric.dumazet, stephan.wurm
Hello:
This patch was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:
On Wed, 29 Jan 2025 13:00:07 +0000 you 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.
>
> [...]
Here is the summary with links:
- [net] net: hsr: fix fill_frame_info() regression vs VLAN packets
https://git.kernel.org/netdev/net/c/0f5697f1a3f9
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2025-01-30 17:10 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-01-29 13:00 [PATCH net] net: hsr: fix fill_frame_info() regression vs VLAN packets Eric Dumazet
2025-01-29 15:13 ` Simon Horman
2025-01-29 15:32 ` Eric Dumazet
2025-01-30 17:10 ` patchwork-bot+netdevbpf
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox