Skip to content

Commit 309a32d

Browse files
committed
net/sched: act_api: use RCU with deferred freeing for action lifecycle
jira VULN-XXXX cve CVE-2026-53264 commit-author Jamal Hadi Salim <jhs@mojatatu.com> commit 5057e1a When NEWTFILTER and DELFILTER are run concurrently it is possible to create a race with an associated action. Let's illustrate with CPU0 running NEWTFILTER and CPU1 running DELFILTER: 0: mutex_lock() <-- holds the idr lock 0: rcu_read_lock() 0: p = idr_find(idr, index) <-- action p is valid (RCU protects IDR) 0: mutex_unlock() <-- releases the idr lock 1: refcount_dec_and_mutex_lock() <-- refcnt 1->0, mutex held 1: idr_remove(idr, index) <-- Action removed from IDR 1: mutex_unlock() <-- mutex released allowing us to delete the action 1: tcf_action_cleanup(p); kfree(p) <-- Kfrees p immediately, no deferral 0: refcount_inc_not_zero(&p->tcfa_refcnt) <-- ouch, UAF p points to freed memory This patch fixes the race condition between NEWTFILTER and DELFILTER by adding struct rcu_head to tc_action used in the deferral and introducing a call_rcu() in the delete path to defer the final kfree(). Note: this is a revert of commit d7fb60b ("net_sched: get rid of tcfa_rcu") but also modernization/simplification to directly use kfree_rcu(). Let's illustrate the new restored code path: 0: rcu_read_lock() 1: refcount_dec_and_mutex_lock() <-- refcnt 1->0, mutex held 1: idr_remove(idr, index) 1: mutex_unlock() 1: call_rcu(&p->tcfa_rcu, tcf_action_rcu_free) <-- defer kfree after grace period 0: p = idr_find(idr, index) 0: refcount_inc_not_zero(&p->tcfa_refcnt) <-- fails, refcnt already 0 1: rcu_read_unlock() <-- release so freeing can run after grace period After CPU1 calls idr_remove(), the object is no longer reachable through the IDR. CPU0's subsequent idr_find() will return NULL, and even if it still held a stale pointer, the immediate kfree() is now deferred until after the RCU grace period, so no UAF can occur. Fixes: d7fb60b ("net_sched: get rid of tcfa_rcu") Suggested-by: Jakub Kicinski <kuba@kernel.org> Reported-by: Kyle Zeng <kylebot@openai.com> Tested-by: Victor Nogueira <victor@mojatatu.com> Tested-by: syzbot@syzkaller.appspotmail.com Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com> Tested-by: Kyle Zeng <kylebot@openai.com> Reviewed-by: Pedro Tammela <pctammela@mojatatu.com> Reviewed-by: Eric Dumazet <edumazet@google.com> Reviewed-by: Victor Nogueira <victor@mojatatu.com> Link: https://patch.msgid.link/20260531160812.68020-1-jhs@mojatatu.com Signed-off-by: Jakub Kicinski <kuba@kernel.org> (cherry picked from commit 5057e1a) Signed-off-by: Brett Mastbergen <bmastbergen@ciq.com>
1 parent d576c0e commit 309a32d

2 files changed

Lines changed: 2 additions & 6 deletions

File tree

include/net/act_api.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ struct tc_action {
4141
struct tc_cookie __rcu *act_cookie;
4242
struct tcf_chain __rcu *goto_chain;
4343
u32 tcfa_flags;
44+
struct rcu_head tcfa_rcu;
4445
u8 hw_stats;
4546
u8 used_hw_stats;
4647
bool used_hw_stats_valid;

net/sched/act_api.c

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -115,11 +115,6 @@ struct tcf_chain *tcf_action_set_ctrlact(struct tc_action *a, int action,
115115
}
116116
EXPORT_SYMBOL(tcf_action_set_ctrlact);
117117

118-
/* XXX: For standalone actions, we don't need a RCU grace period either, because
119-
* actions are always connected to filters and filters are already destroyed in
120-
* RCU callbacks, so after a RCU grace period actions are already disconnected
121-
* from filters. Readers later can not find us.
122-
*/
123118
static void free_tcf(struct tc_action *p)
124119
{
125120
struct tcf_chain *chain = rcu_dereference_protected(p->goto_chain, 1);
@@ -132,7 +127,7 @@ static void free_tcf(struct tc_action *p)
132127
if (chain)
133128
tcf_chain_put_by_act(chain);
134129

135-
kfree(p);
130+
kfree_rcu(p, tcfa_rcu);
136131
}
137132

138133
static void tcf_action_cleanup(struct tc_action *p)

0 commit comments

Comments
 (0)