Re: [PATCH net] net: hsr: avoid potential out-of-bound access in fill_frame_info()
From: Eric Dumazet <edumazet@google.com>
Date: 2025-01-17 18:18:22
On Fri, Jan 17, 2025 at 7:14 PM Eric Dumazet [off-list ref] wrote:
quoted hunk ↗ jump to hunk
On Fri, Jan 17, 2025 at 3:16 PM Stephan Wurm [off-list ref] wrote:quoted
Am 17. Jan 14:22 hat Eric Dumazet geschrieben:quoted
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..b0068e23083416ba13794e3b152517afbe5125b7100644--- 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 modeYup, 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..6f65a535c7fcd740cef81e718323e86fd1eef832100644--- 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 [off-list ref]
Date: Sun Apr 7 08:06:06 2024 +0000
net: display more skb fields in skb_dump()