[PATCH net v2 1/2] net/sched: act_ct: fix sk_buff leak when the header checks reject a packet

[PATCH net v2 1/2] net/sched: act_ct: fix sk_buff leak when the header checks reject a packet

[Hyunjung Ko]

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+
Assisted-by: Anthropic-Claude-Code:Claude-Opus-5
Signed-off-by: Hyunjung Ko <hj351016@gmail.com>
---
 net/sched/act_ct.c | 29 +++++++++++++++++++++++++----
 1 file changed, 25 insertions(+), 4 deletions(-)

v2:
 - add a tdc selftest (patch 2/2), as requested by Jamal
 - add Assisted-by: tag
 - no functional change to the fix itself
v1: https://lore.kernel.org/netdev/20260805095539.204356-1-hj351016@gmail.com/

Reproducer needs CONFIG_NET_ACT_CT, plus CONFIG_DEBUG_KMEMLEAK and
kmemleak=on to observe it:

  ip link add veth0 type veth peer name veth1
  ip link set veth0 up; ip link set veth1 up
  tc qdisc add dev veth0 clsact
  tc filter add dev veth0 ingress matchall action ct

then inject at veth1 a 54-byte frame: ethertype 0x86DD, a 40-byte IPv6
header with nexthdr = 0 (hop-by-hop) and nothing after it, so
ipv6_find_hdr() fails with -EBADMSG and tcf_ct_ipv6_is_fragment()
returns -EPROTO.

Before, one sk_buff plus its data buffer per packet:

  kmemleak: 50 new suspected memory leaks
  unreferenced object 0xffff888103ed13c0 (size 232):
    kmem_cache_alloc_node_noprof+0x2f1/0x3e0
    __alloc_skb+0xe5/0x860
    alloc_skb_with_frags+0x82/0x750
    sock_alloc_send_pskb+0x658/0x7e0
    packet_sendmsg+0x1833/0x4860
    __x64_sys_sendto+0xe0/0x1c0
    do_syscall_64+0x102/0x5a0

After: kmemleak reports no unreferenced objects.

Note /proc/slabinfo is not a usable check here on a KASAN build -
skbuff_head_cache active_objs still grows because the quarantine holds
the freed objects. kmemleak is the reliable signal.

Patch 2/2 turns the same case into a tdc test, using the clsact drop
counter as the discriminator: before the fix act_ct returns
TC_ACT_CONSUMED, so tc_run() never reaches its TC_ACT_SHOT arm and the
counter stays at zero while the skbs leak.

diff --git a/net/sched/act_ct.c b/net/sched/act_ct.c
index be535a261fa0..e250969c84ac 100644
--- 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)
--
2.43.0

[PATCH net v2 2/2] selftests: tc-testing: add act_ct test for malformed header handling

[Hyunjung Ko]

Add a tdc case covering the leak fixed by the previous patch.

The test attaches "action ct" to a clsact ingress chain and injects ten
IPv6 frames whose nexthdr says hop-by-hop but which carry nothing after
the 40-byte header, so ipv6_find_hdr() fails and
tcf_ct_ipv6_is_fragment() returns -EPROTO.

Before the fix act_ct returned TC_ACT_CONSUMED for these packets, so
tc_run() never reached its TC_ACT_SHOT arm and the clsact drop counter
stayed at zero while the skbs leaked. After the fix the packets are
dropped properly and the counter reflects them, which is what the test
matches on:

  before:  Sent 476 bytes 11 pkt (dropped 0, overlimits 0 requeues 0)
  after:   Sent 400 bytes 10 pkt (dropped 10, overlimits 0 requeues 0)

Assisted-by: Anthropic-Claude-Code:Claude-Opus-5
Signed-off-by: Hyunjung Ko <hj351016@gmail.com>
---

 .../selftests/tc-testing/tc-tests/actions/ct.json  | 40 ++++++++++++++++++++++
 1 file changed, 40 insertions(+)

New in v2, requested by Jamal.

Caveat on how far I verified it: I do not have a scapy-capable tdc
environment set up, so I have not run tdc.py over this case itself.
What I did run, on both an unpatched and a patched v7.2-rc6 under
qemu, is exactly what the case does - clsact ingress plus "matchall
action ct" on a veth pair, ten of the same malformed frames injected
on the peer, then "tc -s qdisc show dev <dev> clsact":

  unpatched:  Sent 476 bytes 11 pkt (dropped 0, overlimits 0 requeues 0)
  patched:    Sent 400 bytes 10 pkt (dropped 10, overlimits 0 requeues 0)

so the matchPattern does discriminate. The JSON itself is modelled on
the existing scapy cases in the same file (3992, 9c2a). A run through
tdc.py proper before this is applied would be welcome.

diff --git a/tools/testing/selftests/tc-testing/tc-tests/actions/ct.json b/tools/testing/selftests/tc-testing/tc-tests/actions/ct.json
index da65f838bd52..8ab48def89b6 100644
--- a/tools/testing/selftests/tc-testing/tc-tests/actions/ct.json
+++ b/tools/testing/selftests/tc-testing/tc-tests/actions/ct.json
@@ -702,5 +702,45 @@
             "$TC qdisc del dev $DUMMY clsact",
             "$TC qdisc del dev $DUMMY root handle 1:"
         ]
+    },
+    {
+        "id": "c7a3",
+        "name": "Verify act_ct drops a packet whose header checks fail",
+        "category": [
+            "actions",
+            "ct",
+            "scapy"
+        ],
+        "plugins": {
+            "requires": [
+                "nsPlugin",
+                "scapyPlugin"
+            ]
+        },
+        "setup": [
+            [
+                "$TC qdisc del dev $DEV1 clsact",
+                0,
+                1,
+                2,
+                255
+            ],
+            "$TC qdisc add dev $DEV1 clsact"
+        ],
+        "cmdUnderTest": "$TC filter add dev $DEV1 ingress protocol all prio 1 matchall action ct",
+        "scapy": [
+            {
+                "iface": "$DEV0",
+                "count": 10,
+                "packet": "Ether(type=0x86dd)/IPv6(nh=0, plen=0, src='::1', dst='::2')"
+            }
+        ],
+        "expExitCode": "0",
+        "verifyCmd": "$TC -s qdisc show dev $DEV1 clsact",
+        "matchPattern": "dropped 10",
+        "matchCount": "1",
+        "teardown": [
+            "$TC qdisc del dev $DEV1 clsact"
+        ]
     }
 ]
--
2.43.0

Re: [PATCH net v2 1/2] net/sched: act_ct: fix sk_buff leak when the header checks reject a packet

[Jamal Hadi Salim]

On Thu, Aug 6, 2026 at 6:12 AM Hyunjung Ko <hj351016@gmail.com> wrote:
>
> 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+
> Assisted-by: Anthropic-Claude-Code:Claude-Opus-5
> Signed-off-by: Hyunjung Ko <hj351016@gmail.com>

I actually have tested and reviewed this. So:

Acked-by: Jamal Hadi Salim <jhs@mojatatu.com>

cheers,
jamal
> ---
>  net/sched/act_ct.c | 29 +++++++++++++++++++++++++----
>  1 file changed, 25 insertions(+), 4 deletions(-)
>
> v2:
>  - add a tdc selftest (patch 2/2), as requested by Jamal
>  - add Assisted-by: tag
>  - no functional change to the fix itself
> v1: https://lore.kernel.org/netdev/20260805095539.204356-1-hj351016@gmail.com/
>
> Reproducer needs CONFIG_NET_ACT_CT, plus CONFIG_DEBUG_KMEMLEAK and
> kmemleak=on to observe it:
>
>   ip link add veth0 type veth peer name veth1
>   ip link set veth0 up; ip link set veth1 up
>   tc qdisc add dev veth0 clsact
>   tc filter add dev veth0 ingress matchall action ct
>
> then inject at veth1 a 54-byte frame: ethertype 0x86DD, a 40-byte IPv6
> header with nexthdr = 0 (hop-by-hop) and nothing after it, so
> ipv6_find_hdr() fails with -EBADMSG and tcf_ct_ipv6_is_fragment()
> returns -EPROTO.
>
> Before, one sk_buff plus its data buffer per packet:
>
>   kmemleak: 50 new suspected memory leaks
>   unreferenced object 0xffff888103ed13c0 (size 232):
>     kmem_cache_alloc_node_noprof+0x2f1/0x3e0
>     __alloc_skb+0xe5/0x860
>     alloc_skb_with_frags+0x82/0x750
>     sock_alloc_send_pskb+0x658/0x7e0
>     packet_sendmsg+0x1833/0x4860
>     __x64_sys_sendto+0xe0/0x1c0
>     do_syscall_64+0x102/0x5a0
>
> After: kmemleak reports no unreferenced objects.
>
> Note /proc/slabinfo is not a usable check here on a KASAN build -
> skbuff_head_cache active_objs still grows because the quarantine holds
> the freed objects. kmemleak is the reliable signal.
>
> Patch 2/2 turns the same case into a tdc test, using the clsact drop
> counter as the discriminator: before the fix act_ct returns
> TC_ACT_CONSUMED, so tc_run() never reaches its TC_ACT_SHOT arm and the
> counter stays at zero while the skbs leak.
>
> diff --git a/net/sched/act_ct.c b/net/sched/act_ct.c
> index be535a261fa0..e250969c84ac 100644
> --- 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)
> --
> 2.43.0

Re: [PATCH net v2 2/2] selftests: tc-testing: add act_ct test for malformed header handling

[Victor Nogueira]

On 06/08/2026 07:12, Hyunjung Ko wrote:
> Add a tdc case covering the leak fixed by the previous patch.
> 
> The test attaches "action ct" to a clsact ingress chain and injects ten
> IPv6 frames whose nexthdr says hop-by-hop but which carry nothing after
> the 40-byte header, so ipv6_find_hdr() fails and
> tcf_ct_ipv6_is_fragment() returns -EPROTO.
> 
> Before the fix act_ct returned TC_ACT_CONSUMED for these packets, so
> tc_run() never reached its TC_ACT_SHOT arm and the clsact drop counter
> stayed at zero while the skbs leaked. After the fix the packets are
> dropped properly and the counter reflects them, which is what the test
> matches on:
> 
>    before:  Sent 476 bytes 11 pkt (dropped 0, overlimits 0 requeues 0)
>    after:   Sent 400 bytes 10 pkt (dropped 10, overlimits 0 requeues 0)
> 
> Assisted-by: Anthropic-Claude-Code:Claude-Opus-5
> Signed-off-by: Hyunjung Ko <hj351016@gmail.com>

Reviewed-by: Victor Nogueira <victor@mojatatu.com>

Re: [PATCH net v2 2/2] selftests: tc-testing: add act_ct test for malformed header handling

[Jamal Hadi Salim]

On Thu, Aug 6, 2026 at 2:43 PM Victor Nogueira <victor@mojatatu.com> wrote:
>
> On 06/08/2026 07:12, Hyunjung Ko wrote:
> > Add a tdc case covering the leak fixed by the previous patch.
> >
> > The test attaches "action ct" to a clsact ingress chain and injects ten
> > IPv6 frames whose nexthdr says hop-by-hop but which carry nothing after
> > the 40-byte header, so ipv6_find_hdr() fails and
> > tcf_ct_ipv6_is_fragment() returns -EPROTO.
> >
> > Before the fix act_ct returned TC_ACT_CONSUMED for these packets, so
> > tc_run() never reached its TC_ACT_SHOT arm and the clsact drop counter
> > stayed at zero while the skbs leaked. After the fix the packets are
> > dropped properly and the counter reflects them, which is what the test
> > matches on:
> >
> >    before:  Sent 476 bytes 11 pkt (dropped 0, overlimits 0 requeues 0)
> >    after:   Sent 400 bytes 10 pkt (dropped 10, overlimits 0 requeues 0)
> >
> > Assisted-by: Anthropic-Claude-Code:Claude-Opus-5
> > Signed-off-by: Hyunjung Ko <hj351016@gmail.com>
>
> Reviewed-by: Victor Nogueira <victor@mojatatu.com>

Acked-by: Jamal Hadi Salim <jhs@mojatatu.com>

cheers,
jamal