[PATCH] net/sched: act_ct: fix skb leak on fragment check failure

[PATCH] net/sched: act_ct: fix skb leak on fragment check failure

[Dudu Lu]

tcf_ct_handle_fragments() returns TC_ACT_CONSUMED when
tcf_ct_ipv4/6_is_fragment() fails. This causes the caller to
believe the skb was consumed, but it was not freed. Each
malformed fragment leaks one skb, leading to OOM DoS under
sustained traffic.

Change the return value to TC_ACT_SHOT so the skb is properly
freed by the caller.

Fixes: b57dc7c13ea9 ("net/sched: Introduce action ct")
Signed-off-by: Dudu Lu <phx0fer@gmail.com>
---
 net/sched/act_ct.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/net/sched/act_ct.c b/net/sched/act_ct.c
index 7d5e50c921a0..870655f682bd 100644
--- a/net/sched/act_ct.c
+++ b/net/sched/act_ct.c
@@ -1107,8 +1107,10 @@ TC_INDIRECT_SCOPE int tcf_ct_act(struct sk_buff *skb, const struct tc_action *a,
 	return retval;
 
 out_frag:
-	if (err != -EINPROGRESS)
+	if (err != -EINPROGRESS) {
 		tcf_action_inc_drop_qstats(&c->common);
+		return TC_ACT_SHOT;
+	}
 	return TC_ACT_CONSUMED;
 
 drop:
-- 
2.39.3 (Apple Git-145)

Re: [PATCH] net/sched: act_ct: fix skb leak on fragment check failure

[Simon Horman]

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.

For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
---
net/sched: act_ct: fix skb leak on fragment check failure

This commit fixes a socket buffer leak in the act_ct traffic control action.
When tcf_ct_handle_fragments() encounters malformed fragments and returns an
error, the skb was not being freed, leading to memory exhaustion under
sustained traffic. The fix changes the return value to TC_ACT_SHOT to ensure
proper cleanup.

> net/sched: act_ct: fix skb leak on fragment check failure
>
> tcf_ct_handle_fragments() returns TC_ACT_CONSUMED when
> tcf_ct_ipv4/6_is_fragment() fails. This causes the caller to

The commit message contains a technical inaccuracy. It states
'tcf_ct_handle_fragments() returns TC_ACT_CONSUMED when
tcf_ct_ipv4/6_is_fragment() fails', but tcf_ct_handle_fragments() returns
integer error codes (like -EINVAL, -ENOMEM, -EPROTO), not TC action codes.

The function that returns TC_ACT_CONSUMED is tcf_ct_act(), which interprets
the error codes from tcf_ct_handle_fragments(). The code fix itself is
correct - only the commit message attribution is wrong.

More accurate would be: 'tcf_ct_act() returns TC_ACT_CONSUMED when
tcf_ct_handle_fragments() fails'.

> believe the skb was consumed, but it was not freed. Each
> malformed fragment leaks one skb, leading to OOM DoS under
> sustained traffic.
>
> Change the return value to TC_ACT_SHOT so the skb is properly
> freed by the caller.
>
> Fixes: b57dc7c13ea9 ("net/sched: Introduce action ct")

The existing Fixes: tag points to commit b57dc7c13ea9 ("net/sched: Introduce
action ct"), but the bug was actually introduced by commit 3f14b377d01d
("net/sched: act_ct: fix skb leak and crash on ooo frags").

The original commit b57dc7c13ea9 had correct error handling where fragment
check failures went to the 'drop' label. Commit 3f14b377d01d introduced the
'out_frag' label that always returns TC_ACT_CONSUMED, causing the skb leak
when fragment checks fail with errors other than -EINPROGRESS.

Suggested: Fixes: 3f14b377d01d ("net/sched: act_ct: fix skb leak and crash on ooo frags")

> Signed-off-by: Dudu Lu <phx0fer@gmail.com>
>
> diff --git a/net/sched/act_ct.c b/net/sched/act_ct.c
> index 7d5e50c921a0..870655f682bd 100644
> --- a/net/sched/act_ct.c
> +++ b/net/sched/act_ct.c
> @@ -1107,8 +1107,10 @@ TC_INDIRECT_SCOPE int tcf_ct_act(struct sk_buff *skb, const struct tc_action *a,
>  	return retval;
>
>  out_frag:
> -	if (err != -EINPROGRESS)
> +	if (err != -EINPROGRESS) {
>  		tcf_action_inc_drop_qstats(&c->common);
> +		return TC_ACT_SHOT;
> +	}
>  	return TC_ACT_CONSUMED;
>
>  drop: