Re: [PATCH net 3/4] net/sched: act_api: fix skb sizing and action leak on reoffload delete

[Pedro Tammela <pctammela@mojatatu.com>]

On 24/08/2026 12:39, Victor Nogueira wrote:
> tcf_reoffload_del_notify_msg() sizes the RTM_DELACTION skb with
> tcf_action_fill_size(action) alone.  Unlike every other notification path
> it never wraps that in tcf_action_full_attrs_size(), so the nlmsg_put()
> header, struct tcamsg and the TCA_ACT_TAB nest that tca_get_fill() emits -
> 24 bytes on x86_64 - are not budgeted.  As long as the single action stays
> well under NLMSG_GOODSIZE the floor in alloc_skb() hides this, but once its
> fill size crosses NLMSG_GOODSIZE the allocation is exactly 24 bytes short
> and tca_get_fill() runs out of tailroom.  That is now easy to reach for an
> offloadable act_pedit with a large tcfp_nkeys, which commit 8e2efb3f45a5
> ("net/sched: add get_fill_size callbacks for actions missing them") started
> accounting for properly.
> 
> When that happens tcf_reoffload_del_notify() returns early, before
> tcf_idr_release_unsafe(), and tcf_action_reoffload_cb() discards the return
> value:
> 
> 	if (tc_act_skip_sw(p->tcfa_flags) && !tc_act_in_hw(p))
> 		tcf_reoffload_del_notify(net, p);
> 
> The action has just lost its last hardware instance and is skip_sw, so it
> is left installed while processing no packets, and with no notification to
> tell userspace about it.  An -ENOBUFS from alloc_skb() gets the same
> treatment.
> 
> Fix this by budgeting the message header the way the add and delete paths
> do, and release the action even when the notification cannot be built -
> dropping the notification is strictly better than leaking a dead action,
> and there is no caller left to report the error to.
> 
> Fixes: 13926d19a11e ("flow_offload: add reoffload process to update hw_count")
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Closes: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260810164357.1653956-1-victor%40mojatatu.com
> Acked-by: Jamal Hadi Salim <jhs@mojatatu.com>

Reviewed-by: Pedro Tammela <pctammela@mojatatu.com>

> Signed-off-by: Victor Nogueira <victor@mojatatu.com>
> ---
>   net/sched/act_api.c | 17 +++++++++++------
>   1 file changed, 11 insertions(+), 6 deletions(-)
> 
> diff --git a/net/sched/act_api.c b/net/sched/act_api.c
> index 20b6501fd33b..37eced84dfa5 100644
> --- a/net/sched/act_api.c
> +++ b/net/sched/act_api.c
> @@ -1867,11 +1867,13 @@ static int tcf_action_delete(struct net *net, struct tc_action *actions[])
>   static struct sk_buff *tcf_reoffload_del_notify_msg(struct net *net,
>   						    struct tc_action *action)
>   {
> -	size_t attr_size = tcf_action_fill_size(action);
>   	struct tc_action *actions[TCA_ACT_MAX_PRIO] = {
>   		[0] = action,
>   	};
>   	struct sk_buff *skb;
> +	size_t attr_size;
> +
> +	attr_size = tcf_action_full_attrs_size(tcf_action_fill_size(action));
>   
>   	skb = alloc_skb(max(attr_size, NLMSG_GOODSIZE), GFP_KERNEL);
>   	if (!skb)
> @@ -1888,15 +1890,18 @@ static struct sk_buff *tcf_reoffload_del_notify_msg(struct net *net,
>   static int tcf_reoffload_del_notify(struct net *net, struct tc_action *action)
>   {
>   	const struct tc_action_ops *ops = action->ops;
> -	struct sk_buff *skb;
> +	struct sk_buff *skb = NULL;
>   	int ret;
>   
> -	if (!rtnl_notify_needed(net, 0, RTNLGRP_TC)) {
> -		skb = NULL;
> -	} else {
> +	if (rtnl_notify_needed(net, 0, RTNLGRP_TC)) {
>   		skb = tcf_reoffload_del_notify_msg(net, action);
> +		/* The action has already lost its hardware instance and is
> +		 * skip_sw, so it must be released whether or not the
> +		 * notification can be built.  Drop the notification rather
> +		 * than leave an action behind that processes no packets.
> +		 */
>   		if (IS_ERR(skb))
> -			return PTR_ERR(skb);
> +			skb = NULL;
>   	}
>   
>   	ret = tcf_idr_release_unsafe(action);