Re: [PATCH net v2] net/sched: act_gact, act_police: range check the fallback control action

[Victor Nogueira]

On 06/08/2026 07:12, Hyunjung Ko wrote:
> tcf_action_check_ctrlact() range checks the primary control action:
>
> 	if (!opcode)
> 		ret = action > TC_ACT_VALUE_MAX ? -EINVAL : 0;
>
> TC_ACT_VALUE_MAX is TC_ACT_TRAP, so kernel-internal verdicts above it
> cannot be set that way. But act_gact and act_police each carry a second,
> independent control action supplied by user space that never reaches that
> helper - TCA_GACT_PROB.paction and TCA_POLICE_RESULT. Both only reject
> TC_ACT_GOTO_CHAIN, so any other value is stored verbatim and returned
> verbatim from the action.
>
> In particular user space can store TC_ACT_CONSUMED, which is
> TC_ACT_VALUE_MAX + 1 and is deliberately not part of the UAPI value
> range. That verdict tells every caller the action took ownership of the
> skb, so nobody frees it: sch_handle_ingress(), sch_handle_egress() and
> tcf_qevent_handle() all deliberately skip the free for it. The result is
> one leaked sk_buff plus its data buffer per packet traversing the filter,
> unbounded, for all traffic on the chain including kernel-generated
> packets.
>
> Both are trivially deterministic. act_gact clamps tcfg_pval to >= 1, so
> with pval = 1 gact_determ() returns the fallback for every packet.
> act_police has no mandatory rate, so rate = 0 leaves tcfp_mtu = ~0 and
> tcf_police_mtu_check() always passes.
>
> TC_ACT_CONSUMED was added by commit 720f22fed81b ("net: sched: refactor
> reinsert action"), after both goto-chain guards were written:
> commit 9469f375ab09 ("net/sched: act_gact: disallow 'goto chain' on
> fallback control action") and
> commit c08f5ed5d625 ("net/sched: act_police: disallow 'goto chain' on
> fallback control action"). Neither guard was widened when the new
> verdict appeared.
>
> Factor the existing range test out of tcf_action_check_ctrlact() as
> tcf_action_valid() and apply it to both fallbacks. The helper cannot call
> tcf_action_check_ctrlact() directly because that also allocates a
> goto_chain, which is exactly what these two sites must not do.
>
> Reproduced on v7.2-rc6: kmemleak reports one leaked 232-byte
> skbuff_head_cache object plus its 704-byte data buffer per packet. With
> this patch both configurations are rejected with -EINVAL and kmemleak
> reports none.
>
> Fixes: 720f22fed81b ("net: sched: refactor reinsert action")
> Cc: stable@xxxxxxxxxxxxxxx # v5.3+
> Assisted-by: Anthropic-Claude-Code:Claude-Opus-5
> Signed-off-by: Hyunjung Ko <hj351016@xxxxxxxxx>

Tested-by: Victor Nogueira <victor@xxxxxxxxxxxx>