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

[Jamal Hadi Salim]

On Tue, May 26, 2026 at 6:09 PM Kyle Zeng <kylebot@xxxxxxxxxx> 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@xxxxxxxxxx>

Acked-by: Jamal Hadi Salim <jhs@xxxxxxxxxxxx>

cheers,
jamal

> ---
>  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 */
> --
> 2.54.0
>