Re: [PATCH net] net/sched: act_api: use mutex in tcf_idr_check_alloc

[Pedro Tammela <pctammela@mojatatu.com>]

On 26/05/2026 19:08, Kyle Zeng wrote:
> Currently, the NEWTFILTER path uses RCU to guard action idr accesses while
> the DELTFILTER path uses mutex to guard action accesses. This
> inconsistency leads to a race condition scenario, which can lead to
> erroneous operations on refcount, eventually leading to use-after-free
> situation.
> In this patch, we revert the introduction of RCU back to mutex in the
> NEWFILTER path, which is consistent with the DELFILTER path, avoiding
> the race condition.
> 
> Fixes: 4b55e86736d5 ("net/sched: act_api: rely on rcu in tcf_idr_check_alloc")
> Signed-off-by: Kyle Zeng <kylebot@openai.com>

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

> ---
>   net/sched/act_api.c | 12 +++++-------
>   1 file changed, 5 insertions(+), 7 deletions(-)
> 
> diff --git a/net/sched/act_api.c b/net/sched/act_api.c
> index 332fd9695e54..63841c3e4310 100644
> --- a/net/sched/act_api.c
> +++ b/net/sched/act_api.c
> @@ -874,36 +874,34 @@ int tcf_idr_check_alloc(struct tc_action_net *tn, u32 *index,
>   	u32 max;
>   
>   	if (*index) {
> -		rcu_read_lock();
> +		mutex_lock(&idrinfo->lock);
>   		p = idr_find(&idrinfo->action_idr, *index);
>   
>   		if (IS_ERR(p)) {
>   			/* This means that another process allocated
>   			 * index but did not assign the pointer yet.
>   			 */
> -			rcu_read_unlock();
> +			mutex_unlock(&idrinfo->lock);
>   			return -EAGAIN;
>   		}
>   
>   		if (!p) {
>   			/* Empty slot, try to allocate it */
>   			max = *index;
> -			rcu_read_unlock();
> +			mutex_unlock(&idrinfo->lock);
>   			goto new;
>   		}
>   
>   		if (!refcount_inc_not_zero(&p->tcfa_refcnt)) {
>   			/* Action was deleted in parallel */
> -			rcu_read_unlock();
> +			mutex_unlock(&idrinfo->lock);
>   			return -EAGAIN;
>   		}
>   
>   		if (bind)
>   			atomic_inc(&p->tcfa_bindcnt);
>   		*a = p;
> -
> -		rcu_read_unlock();
> -
> +		mutex_unlock(&idrinfo->lock);
>   		return 1;
>   	} else {
>   		/* Find a slot */