mirror of
https://github.com/LineageOS/android_kernel_fxtec_sm6115.git
synced 2026-08-18 11:41:01 +00:00
net/sched: act_api: use RCU with deferred freeing for action lifecycle
[ Upstream commit 5057e1aca011e51ef51498c940ef96f3d3e8a305 ] 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 commitd7fb60b9ca("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:d7fb60b9ca("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> Signed-off-by: Sasha Levin <sashal@kernel.org> [uli: backport to 4.19] Signed-off-by: Ulrich Hecht <uli@kernel.org> Reviewed-by: Pavel Machek <pavel@nabladev.com>
This commit is contained in:
committed by
Ulrich Hecht
parent
9bb956133b
commit
0c23ec2e23
@ -39,6 +39,7 @@ struct tc_action {
|
||||
struct gnet_stats_queue __percpu *cpu_qstats;
|
||||
struct tc_cookie __rcu *act_cookie;
|
||||
struct tcf_chain *goto_chain;
|
||||
struct rcu_head tcfa_rcu;
|
||||
};
|
||||
#define tcf_index common.tcfa_index
|
||||
#define tcf_refcnt common.tcfa_refcnt
|
||||
|
||||
@ -73,11 +73,6 @@ static void tcf_set_action_cookie(struct tc_cookie __rcu **old_cookie,
|
||||
call_rcu(&old->rcu, tcf_free_cookie_rcu);
|
||||
}
|
||||
|
||||
/* XXX: For standalone actions, we don't need a RCU grace period either, because
|
||||
* actions are always connected to filters and filters are already destroyed in
|
||||
* RCU callbacks, so after a RCU grace period actions are already disconnected
|
||||
* from filters. Readers later can not find us.
|
||||
*/
|
||||
static void free_tcf(struct tc_action *p)
|
||||
{
|
||||
free_percpu(p->cpu_bstats);
|
||||
@ -87,7 +82,7 @@ static void free_tcf(struct tc_action *p)
|
||||
if (p->goto_chain)
|
||||
tcf_action_goto_chain_fini(p);
|
||||
|
||||
kfree(p);
|
||||
kfree_rcu(p, tcfa_rcu);
|
||||
}
|
||||
|
||||
static void tcf_action_cleanup(struct tc_action *p)
|
||||
|
||||
Reference in New Issue
Block a user