[PATCH net] net/sched: fix use-after-free in __tcf_action_put()

[PATCH net] net/sched: fix use-after-free in __tcf_action_put()

[Jérémy Jean]

__tcf_action_put() decrements the reference count before decrementing the
binding count. Another thread can free the action between these operations,
causing the binding-count update to access freed memory. KASAN reports it
as:

  BUG: KASAN: slab-use-after-free in __tcf_action_put+0x327/0x340
  Write of size 4 at addr ff11000000d46420 by task tc-put-pause/64

Hold the action IDR mutex across both counter updates and IDR removal,
keeping cleanup outside the lock.

Fixes: 16af6067392c ("net: sched: implement reference counted action release")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Jérémy Jean <Jeremy.Jean@oss.cyber.gouv.fr>
---
 net/sched/act_api.c | 19 +++++++++----------
 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index e45a63be39..a422bc7 100644
--- a/net/sched/act_api.c
+++ b/net/sched/act_api.c
@@ -372,20 +372,19 @@ static int __tcf_action_put(struct tc_action *p, bool bind)
 {
 	struct tcf_idrinfo *idrinfo = p->idrinfo;
 
-	if (refcount_dec_and_mutex_lock(&p->tcfa_refcnt, &idrinfo->lock)) {
-		if (bind)
-			atomic_dec(&p->tcfa_bindcnt);
-		idr_remove(&idrinfo->action_idr, p->tcfa_index);
+	mutex_lock(&idrinfo->lock);
+	if (bind)
+		atomic_dec(&p->tcfa_bindcnt);
+	if (!refcount_dec_and_test(&p->tcfa_refcnt)) {
 		mutex_unlock(&idrinfo->lock);
-
-		tcf_action_cleanup(p);
-		return 1;
+		return 0;
 	}
 
-	if (bind)
-		atomic_dec(&p->tcfa_bindcnt);
+	idr_remove(&idrinfo->action_idr, p->tcfa_index);
+	mutex_unlock(&idrinfo->lock);
 
-	return 0;
+	tcf_action_cleanup(p);
+	return 1;
 }
 
 static int __tcf_idr_release(struct tc_action *p, bool bind, bool strict)
-- 
2.47.3

Re: [PATCH net] net/sched: fix use-after-free in __tcf_action_put()

[netdev-bot+sinfo]

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - How the issue was discovered, e.g. hit in production, hit during
   development, syzbot report, manual code inspection, LLM or static
   analysis tool scan.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.