net/sched: act_ct: fix sk_buff leak when the header checks reject a packet

tcf_ct_handle_fragments() runs its header sanity checks before handing
anything to the defragmentation engine:

	if (family == NFPROTO_IPV4)
		err = tcf_ct_ipv4_is_fragment(skb, &frag);
	else
		err = tcf_ct_ipv6_is_fragment(skb, &frag);
	if (err || !frag)
		return err;

tcf_ct_ipv4_is_fragment() returns -EINVAL or -ENOMEM;
tcf_ct_ipv6_is_fragment() adds -EPROTO when ipv6_find_hdr() fails. None of
them frees or queues the skb, so on that path the caller still owns it.

tcf_ct_act() however funnels every non-zero return into the
ownership-transfer exit:

	err = tcf_ct_handle_fragments(net, skb, family, p->zone, &defrag);
	if (err)
		goto out_frag;
	...
out_frag:
	if (err != -EINPROGRESS)
		tcf_action_inc_drop_qstats(&c->common);
	return TC_ACT_CONSUMED;

TC_ACT_CONSUMED means the action took ownership of the skb, so no caller
frees it - sch_handle_ingress(), sch_handle_egress() and
tcf_qevent_handle() all deliberately skip the free for that verdict. The
skb is therefore orphaned: one sk_buff plus its data buffer is leaked per
malformed packet, unbounded. Note the drop counter is already incremented
for these errors, so the statistics claim a drop that never happens.

Three different ownership states reach out_frag: today - the skb may be
queued by the defrag engine (-EINPROGRESS), already freed by
nf_ct_handle_fragments(), or still owned by us. Tell the caller which of
those it is, and free the packet ourselves in the last case, which
restores the TC_ACT_SHOT behaviour that predated the Fixes: commit.

Reproduced on v7.2-rc6 with a 54-byte frame carrying a 40-byte IPv6
header with nexthdr = 0 (hop-by-hop) and nothing after it, on a
clsact ingress chain with "action ct". kmemleak reports one leaked
232-byte skbuff_head_cache object plus its 704-byte data buffer per
packet; with this patch it reports none.

Fixes: 3f14b377d01d ("net/sched: act_ct: fix skb leak and crash on ooo frags")
Cc: stable@vger.kernel.org # v6.8+
Signed-off-by: Hyunjung Ko <hj351016@gmail.com>
Acked-by: Jamal Hadi Salim <jhs@mojatatu.com>
Link: https://patch.msgid.link/20260806101235.809370-1-hj351016@gmail.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>

diff --git a/net/sched/act_ct.c b/net/sched/act_ct.c
index be535a261fa04a73a7d005b7c24b38161b3196e0..e250969c84aca41604c40ef678ebe2fe555fb319 100644 (file)
--- a/net/sched/act_ct.c
+++ b/net/sched/act_ct.c
@@ -840,8 +840,15 @@ static int tcf_ct_ipv6_is_fragment(struct sk_buff *skb, bool *frag)
        return 0;
 }
 
+/* On error, tells the caller whether it still owns @skb and must free it
+ * itself.  @skb is ours only when the header checks below reject the packet
+ * before it is handed to the defragmentation engine; once nf_ct_handle_
+ * fragments() has been called the skb is either queued (-EINPROGRESS) or has
+ * already been freed by it.
+ */
 static int tcf_ct_handle_fragments(struct net *net, struct sk_buff *skb,
-                                  u8 family, u16 zone, bool *defrag)
+                                  u8 family, u16 zone, bool *defrag,
+                                  bool *skb_is_ours)
 {
        enum ip_conntrack_info ctinfo;
        struct tc_skb_cb cb;
@@ -859,8 +866,12 @@ static int tcf_ct_handle_fragments(struct net *net, struct sk_buff *skb,
                err = tcf_ct_ipv4_is_fragment(skb, &frag);
        else
                err = tcf_ct_ipv6_is_fragment(skb, &frag);
-       if (err || !frag)
+       if (err) {
+               *skb_is_ours = true;
                return err;
+       }
+       if (!frag)
+               return 0;
 
        cb = *tc_skb_cb(skb);
        err = nf_ct_handle_fragments(net, skb, zone, family, &proto, &cb.mru);
@@ -977,6 +988,7 @@ TC_INDIRECT_SCOPE int tcf_ct_act(struct sk_buff *skb, const struct tc_action *a,
        int nh_ofs, err, retval;
        struct tcf_ct_params *p;
        bool add_helper = false;
+       bool skb_is_ours = false;
        bool skip_add = false;
        bool defrag = false;
        struct nf_conn *ct;
@@ -1012,9 +1024,18 @@ TC_INDIRECT_SCOPE int tcf_ct_act(struct sk_buff *skb, const struct tc_action *a,
         */
        nh_ofs = skb_network_offset(skb);
        skb_pull_rcsum(skb, nh_ofs);
-       err = tcf_ct_handle_fragments(net, skb, family, p->zone, &defrag);
-       if (err)
+       err = tcf_ct_handle_fragments(net, skb, family, p->zone, &defrag,
+                                     &skb_is_ours);
+       if (err) {
+               /* The skb is still ours only when the header checks rejected
+                * it; returning TC_ACT_CONSUMED for such a packet would leak
+                * it, since no caller frees an skb it was told it no longer
+                * owns.
+                */
+               if (skb_is_ours)
+                       goto drop;
                goto out_frag;
+       }
 
        err = nf_ct_skb_network_trim(skb, family);
        if (err)