From 60ce8c483fb5969258814b670f322dd63f41f93c Mon Sep 17 00:00:00 2001 From: Greg Kroah-Hartman Date: Fri, 22 Aug 2025 08:15:55 +0200 Subject: [PATCH] 5.10-stable patches added patches: net-hsr-reject-hsr-frame-if-skb-can-t-hold-tag.patch --- ...ject-hsr-frame-if-skb-can-t-hold-tag.patch | 189 ++++++++++++++++++ queue-5.10/series | 1 + 2 files changed, 190 insertions(+) create mode 100644 queue-5.10/net-hsr-reject-hsr-frame-if-skb-can-t-hold-tag.patch diff --git a/queue-5.10/net-hsr-reject-hsr-frame-if-skb-can-t-hold-tag.patch b/queue-5.10/net-hsr-reject-hsr-frame-if-skb-can-t-hold-tag.patch new file mode 100644 index 0000000000..339b60b0e0 --- /dev/null +++ b/queue-5.10/net-hsr-reject-hsr-frame-if-skb-can-t-hold-tag.patch @@ -0,0 +1,189 @@ +From 7af76e9d18a9fd6f8611b3313c86c190f9b6a5a7 Mon Sep 17 00:00:00 2001 +From: Jakub Acs +Date: Tue, 19 Aug 2025 08:28:42 +0000 +Subject: net, hsr: reject HSR frame if skb can't hold tag +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +From: Jakub Acs + +commit 7af76e9d18a9fd6f8611b3313c86c190f9b6a5a7 upstream. + +Receiving HSR frame with insufficient space to hold HSR tag in the skb +can result in a crash (kernel BUG): + +[ 45.390915] skbuff: skb_under_panic: text:ffffffff86f32cac len:26 put:14 head:ffff888042418000 data:ffff888042417ff4 tail:0xe end:0x180 dev:bridge_slave_1 +[ 45.392559] ------------[ cut here ]------------ +[ 45.392912] kernel BUG at net/core/skbuff.c:211! +[ 45.393276] Oops: invalid opcode: 0000 [#1] SMP DEBUG_PAGEALLOC KASAN NOPTI +[ 45.393809] CPU: 1 UID: 0 PID: 2496 Comm: reproducer Not tainted 6.15.0 #12 PREEMPT(undef) +[ 45.394433] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.16.3-0-ga6ed6b701f0a-prebuilt.qemu.org 04/01/2014 +[ 45.395273] RIP: 0010:skb_panic+0x15b/0x1d0 + + + +[ 45.402911] Call Trace: +[ 45.403105] +[ 45.404470] skb_push+0xcd/0xf0 +[ 45.404726] br_dev_queue_push_xmit+0x7c/0x6c0 +[ 45.406513] br_forward_finish+0x128/0x260 +[ 45.408483] __br_forward+0x42d/0x590 +[ 45.409464] maybe_deliver+0x2eb/0x420 +[ 45.409763] br_flood+0x174/0x4a0 +[ 45.410030] br_handle_frame_finish+0xc7c/0x1bc0 +[ 45.411618] br_handle_frame+0xac3/0x1230 +[ 45.413674] __netif_receive_skb_core.constprop.0+0x808/0x3df0 +[ 45.422966] __netif_receive_skb_one_core+0xb4/0x1f0 +[ 45.424478] __netif_receive_skb+0x22/0x170 +[ 45.424806] process_backlog+0x242/0x6d0 +[ 45.425116] __napi_poll+0xbb/0x630 +[ 45.425394] net_rx_action+0x4d1/0xcc0 +[ 45.427613] handle_softirqs+0x1a4/0x580 +[ 45.427926] do_softirq+0x74/0x90 +[ 45.428196] + +This issue was found by syzkaller. + +The panic happens in br_dev_queue_push_xmit() once it receives a +corrupted skb with ETH header already pushed in linear data. When it +attempts the skb_push() call, there's not enough headroom and +skb_push() panics. + +The corrupted skb is put on the queue by HSR layer, which makes a +sequence of unintended transformations when it receives a specific +corrupted HSR frame (with incomplete TAG). + +Fix it by dropping and consuming frames that are not long enough to +contain both ethernet and hsr headers. + +Alternative fix would be to check for enough headroom before skb_push() +in br_dev_queue_push_xmit(). + +In the reproducer, this is injected via AF_PACKET, but I don't easily +see why it couldn't be sent over the wire from adjacent network. + +Further Details: + +In the reproducer, the following network interface chain is set up: + +┌────────────────┐ ┌────────────────┐ +│ veth0_to_hsr ├───┤ hsr_slave0 ┼───┐ +└────────────────┘ └────────────────┘ │ + │ ┌──────┐ + ├─┤ hsr0 ├───┐ + │ └──────┘ │ +┌────────────────┐ ┌────────────────┐ │ │┌────────┐ +│ veth1_to_hsr ┼───┤ hsr_slave1 ├───┘ └┤ │ +└────────────────┘ └────────────────┘ ┌┼ bridge │ + ││ │ + │└────────┘ + │ + ┌───────┐ │ + │ ... ├──────┘ + └───────┘ + +To trigger the events leading up to crash, reproducer sends a corrupted +HSR frame with incomplete TAG, via AF_PACKET socket on 'veth0_to_hsr'. + +The first HSR-layer function to process this frame is +hsr_handle_frame(). It and then checks if the +protocol is ETH_P_PRP or ETH_P_HSR. If it is, it calls +skb_set_network_header(skb, ETH_HLEN + HSR_HLEN), without checking that +the skb is long enough. For the crashing frame it is not, and hence the +skb->network_header and skb->mac_len fields are set incorrectly, +pointing after the end of the linear buffer. + +I will call this a BUG#1 and it is what is addressed by this patch. In +the crashing scenario before the fix, the skb continues to go down the +hsr path as follows. + +hsr_handle_frame() then calls this sequence +hsr_forward_skb() + fill_frame_info() + hsr->proto_ops->fill_frame_info() + hsr_fill_frame_info() + +hsr_fill_frame_info() contains a check that intends to check whether the +skb actually contains the HSR header. But the check relies on the +skb->mac_len field which was erroneously setup due to BUG#1, so the +check passes and the execution continues back in the hsr_forward_skb(): + +hsr_forward_skb() + hsr_forward_do() + hsr->proto_ops->get_untagged_frame() + hsr_get_untagged_frame() + create_stripped_skb_hsr() + +In create_stripped_skb_hsr(), a copy of the skb is created and is +further corrupted by operation that attempts to strip the HSR tag in a +call to __pskb_copy(). + +The skb enters create_stripped_skb_hsr() with ethernet header pushed in +linear buffer. The skb_pull(skb_in, HSR_HLEN) thus pulls 6 bytes of +ethernet header into the headroom, creating skb_in with a headroom of +size 8. The subsequent __pskb_copy() then creates an skb with headroom +of just 2 and skb->len of just 12, this is how it looks after the copy: + +gdb) p skb->len +$10 = 12 +(gdb) p skb->data +$11 = (unsigned char *) 0xffff888041e45382 "\252\252\252\252\252!\210\373", +(gdb) p skb->head +$12 = (unsigned char *) 0xffff888041e45380 "" + +It seems create_stripped_skb_hsr() assumes that ETH header is pulled +in the headroom when it's entered, because it just pulls HSR header on +top. But that is not the case in our code-path and we end up with the +corrupted skb instead. I will call this BUG#2 + +*I got confused here because it seems that under no conditions can +create_stripped_skb_hsr() work well, the assumption it makes is not true +during the processing of hsr frames - since the skb_push() in +hsr_handle_frame to skb_pull in hsr_deliver_master(). I wonder whether I +missed something here.* + +Next, the execution arrives in hsr_deliver_master(). It calls +skb_pull(ETH_HLEN), which just returns NULL - the SKB does not have +enough space for the pull (as it only has 12 bytes in total at this +point). + +*The skb_pull() here further suggests that ethernet header is meant +to be pushed through the whole hsr processing and +create_stripped_skb_hsr() should pull it before doing the HSR header +pull.* + +hsr_deliver_master() then puts the corrupted skb on the queue, it is +then picked up from there by bridge frame handling layer and finally +lands in br_dev_queue_push_xmit where it panics. + +Cc: stable@kernel.org +Fixes: 48b491a5cc74 ("net: hsr: fix mac_len checks") +Reported-by: syzbot+a81f2759d022496b40ab@syzkaller.appspotmail.com +Signed-off-by: Jakub Acs +Reviewed-by: Eric Dumazet +Link: https://patch.msgid.link/20250819082842.94378-1-acsjakub@amazon.de +Signed-off-by: Jakub Kicinski +Signed-off-by: Greg Kroah-Hartman +--- + net/hsr/hsr_slave.c | 8 +++++++- + 1 file changed, 7 insertions(+), 1 deletion(-) + +--- a/net/hsr/hsr_slave.c ++++ b/net/hsr/hsr_slave.c +@@ -60,8 +60,14 @@ static rx_handler_result_t hsr_handle_fr + skb_push(skb, ETH_HLEN); + skb_reset_mac_header(skb); + if ((!hsr->prot_version && protocol == htons(ETH_P_PRP)) || +- protocol == htons(ETH_P_HSR)) ++ protocol == htons(ETH_P_HSR)) { ++ if (!pskb_may_pull(skb, ETH_HLEN + HSR_HLEN)) { ++ kfree_skb(skb); ++ goto finish_consume; ++ } ++ + skb_set_network_header(skb, ETH_HLEN + HSR_HLEN); ++ } + skb_reset_mac_len(skb); + + hsr_forward_skb(skb, port); diff --git a/queue-5.10/series b/queue-5.10/series index 1ce0df64af..c9c793ff0f 100644 --- a/queue-5.10/series +++ b/queue-5.10/series @@ -384,3 +384,4 @@ media-rainshadow-cec-fix-toctou-race-condition-in-rain_interrupt.patch media-ov2659-fix-memory-leaks-in-ov2659_probe.patch media-venus-add-a-check-for-packet-size-after-reading-from-shared-memory.patch drm-amd-restore-cached-power-limit-during-resume.patch +net-hsr-reject-hsr-frame-if-skb-can-t-hold-tag.patch -- 2.47.3