Re: [PATCH net] net/sched: sch_frag: reject a negative L2 length in sch_fragment()
From: Jamal Hadi Salim <jhs@mojatatu.com>
Date: 2026-08-28 10:43:57
On Thu, Aug 27, 2026 at 5:24 AM Fourie Zhang [off-list ref] wrote:
sch_fragment() bounds the L2 header length only from above:
if (skb_network_offset(skb) > VLAN_ETH_HLEN)
skb_network_offset() returns int and the comparison is signed, so a
negative offset passes. sch_frag_prepare_frag() then stores it in an
unsigned int and uses it as a memcpy() length into the 18-byte per-CPU
l2_data buffer:The patch looks good. Comments: 1. Cc: stable@kernel.org should be Cc: stable@vger.kernel.org. 2. Looking at the current sashiko review, the two issues it mentions certainly do not require a v2. A followup is needed. I am recording the followups - do you want to send thos patches? Otherwise we will. 3. In the future, if you send a patch make sure you include a tdc test that would fail without your patch and pass with your patch.We test everything tc related. Can you provide a tdc test or a standalone reproducer script? If it is sensitive just send directly to me. Handwaving: you dont need a v2 except for #1 above. Unfortunately, it seems the nipa sashiko now looks only 72 hours later, so you are not off the hook yet. Lets wait to see what the overlord says before deciding if there's a need for v2. cheers, jamal
quoted hunk ↗ jump to hunk
unsigned int hlen = skb_network_offset(skb); memcpy(&data->l2_data, skb->data, hlen); The offset is negative whenever the network header sits behind skb->data. ipv6_srh_rcv() runs in that state, because ip6_protocol_deliver_rcu() pulls each extension header as it walks the chain and the segments_left > 0 branch pushes back only sizeof(struct ipv6hdr). act_ct enables the static key that gates sch_frag_xmit_hook() and sets tc_skb_cb(skb)->mru, which survives receive and forward, so an act_mirred redirect from a root qdisc can reach sch_fragment() with the offset still negative. A negative L2 header length is invalid, so reject it through the existing "L2 header too long to fragment" path. A non-negative offset behaves exactly as before. Reproduced on v7.2 by an unprivileged user with only unshare(CLONE_NEWUSER|CLONE_NEWNET). RDX is the truncated length, (unsigned int)(-42): RIP: 0010:memcpy+0x8/0x20 RDX: 00000000ffffffd6 sch_frag_prepare_frag+0x364/0x440 net/sched/sch_frag.c:74 sch_fragment+0x1c7/0x7a0 net/sched/sch_frag.c:122 tcf_mirred_to_dev+0x7a9/0xf10 ip6_forward+0x114b/0x31b0 ipv6_rthdr_rcv+0x40d7/0x5e10 With the patch applied the drop path fires and the packet is discarded. Fixes: c129412f74e9 ("net/sched: sch_frag: add generic packet fragment support.") Cc: stable@kernel.org Reported-by: TencentOS Corvus AI <redacted> Assisted-by: tencentos-corvus-ai:kimi-k3 Signed-off-by: Fourie Zhang <redacted> --- net/sched/sch_frag.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)diff --git a/net/sched/sch_frag.c b/net/sched/sch_frag.c index 75ee52750919..b68a198b1646 100644 --- a/net/sched/sch_frag.c +++ b/net/sched/sch_frag.c@@ -89,9 +89,10 @@ static struct dst_ops sch_frag_dst_ops = { static int sch_fragment(struct net *net, struct sk_buff *skb, u16 mru, int (*xmit)(struct sk_buff *skb)) { + int hlen = skb_network_offset(skb); int ret = -1; - if (skb_network_offset(skb) > VLAN_ETH_HLEN) { + if (hlen < 0 || hlen > VLAN_ETH_HLEN) { net_warn_ratelimited("L2 header too long to fragment\n"); goto err; } --2.43.7