Re: [PATCH net] net/sched: sch_frag: reject a negative L2 length in sch_fragment()

On Fri, Aug 28, 2026 at 12:43 PM Jamal Hadi Salim [off-list ref] wrote:

On Thu, Aug 27, 2026 at 5:24 AM Fourie Zhang [off-list ref] wrote:

quoted

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

        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;
        }

I am only speaking for mysef, but

pw-bot: cr

IMO this is defensive programming working around the real bug in ipv6_srh_rcv().

Let's fix the root cause.

I saw a recent patch which had some issues:
https://sashiko.dev/#/patchset/20260817104128.22681-1-juny24602@gmail.com?part=1

It's funny that both patches were assisted by tencentos-corvus-ai:kimi-k3 :/

I think something like this could fix both issues:

diff --git a/net/ipv6/exthdrs.c b/net/ipv6/exthdrs.c
index 51941ad656a36e739388c7da2fcd639ab0e453b5..09a4552f7f08aa8208eb981c0536c58a3561ecbd

100644

--- a/net/ipv6/exthdrs.c
+++ b/net/ipv6/exthdrs.c

@@ -445,7 +445,7 @@ static int ipv6_srh_rcv(struct sk_buff *skb,

struct inet6_dev *idev)
        hdr->segments_left--;
        addr = hdr->segments + hdr->segments_left;

-       skb_push(skb, sizeof(struct ipv6hdr));
+       skb_push(skb, -skb_network_offset(skb));

        if (skb->ip_summed == CHECKSUM_COMPLETE)
                seg6_update_csum(skb);

@@ -469,7 +469,7 @@ static int ipv6_srh_rcv(struct sk_buff *skb,

struct inet6_dev *idev)
                }
                ipv6_hdr(skb)->hop_limit--;

-               skb_pull(skb, sizeof(struct ipv6hdr));
+               skb_pull(skb, skb_transport_offset(skb));
                goto looped_back;
        }