Re: [PATCH net 1/1] net/sched: cls_route: fix fastmap use-after-free on filter
From: Jamal Hadi Salim <jhs@mojatatu.com>
Date: 2026-07-28 12:37:35
Also in:
stable
On Tue, Jul 28, 2026 at 6:34 AM Paolo Abeni [off-list ref] wrote:
On 7/23/26 12:52 PM, Jamal Hadi Salim wrote:quoted
diff --git a/net/sched/cls_route.c b/net/sched/cls_route.c index bd6f945bd388..700f878fc3cd 100644 --- a/net/sched/cls_route.c +++ b/net/sched/cls_route.c@@ -297,16 +297,29 @@ static void route4_destroy(struct tcf_proto *tp, bool rtnl_held, next = rtnl_dereference(f->next); RCU_INIT_POINTER(b->ht[h2], next); tcf_unbind_filter(tp, &f->res); - if (tcf_exts_get_net(&f->exts)) - route4_queue_work(f); - else - __route4_delete_filter(f); + /* Always defer the free. The direct + * __route4_delete_filter() path has no + * grace period and races with readers + * that cached f in the fastmap. + */ + tcf_exts_get_net(&f->exts); + route4_queue_work(f);Sashiko fears the above would be race prone: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260723105210.817079-1-jhs%40mojatatu.com
That was fixed in v2. Is there something i can do next time (message patchwork?) to not waste your time looking at an older version?
quoted
} } RCU_INIT_POINTER(head->table[h1], NULL); kfree_rcu(b, rcu); } } + + /* All filters are unlinked; no new reader can find them on the + * chain. Wait for in-flight readers that may still hold a filter + * pointer and have published it into the fastmap after we unlinked. + * Then flush the stale entries while head is still valid under + * RTNL, matching the route4_delete() pattern. + */ + synchronize_rcu(); + route4_reset_fastmap(head);This really looks like quite a big hammer.
It is. I viewed it as bug fix to original intent of 1109c00547fc - that commit even though had the intent of protecting the fast map as as well (based on the wording of comment i saw). If performance is a big concern: another approach that maybe worth considering is to totally remove the fastpath. It maybe more performant than imposing the rcus.
Since there is already a synchronization point for both reader and write while acquiring the fastmap_lock, I'm wondering if something alike the following could work ?!? (completely untested!!!):
Your approach has some holes, example misses the most dangerous part - destroy(), etc I will mull over it and see if it can be fixed to handle all the issues. I think it's possible i just need to put time to validate/test. cheers, jamal
quoted hunk ↗ jump to hunk
---diff --git a/net/sched/cls_route.c b/net/sched/cls_route.c index bd6f945bd388..8cecc945dcde 100644 --- a/net/sched/cls_route.c +++ b/net/sched/cls_route.c@@ -52,6 +52,7 @@ struct route4_filter { struct tcf_result res; struct tcf_exts exts; u32 handle; + bool dying; struct route4_bucket *bkt; struct tcf_proto *tp; struct rcu_work rwork;@@ -66,9 +67,11 @@ static inline int route4_fastmap_hash(u32 id, int iif) static DEFINE_SPINLOCK(fastmap_lock); static void -route4_reset_fastmap(struct route4_head *head) +route4_reset_fastmap(struct route4_head *head, struct route4_filter *f) { spin_lock_bh(&fastmap_lock); + if (f) + f->dying = true; memset(head->fastmap, 0, sizeof(head->fastmap)); spin_unlock_bh(&fastmap_lock); }@@ -81,9 +84,11 @@ route4_set_fastmap(struct route4_head *head, u32 id, int iif, /* fastmap updates must look atomic to aling id, iff, filter */ spin_lock_bh(&fastmap_lock); - head->fastmap[h].id = id; - head->fastmap[h].iif = iif; - head->fastmap[h].filter = f; + if (!f->dying) { + head->fastmap[h].id = id; + head->fastmap[h].iif = iif; + head->fastmap[h].filter = f; + } spin_unlock_bh(&fastmap_lock); }@@ -338,7 +343,7 @@ static int route4_delete(struct tcf_proto *tp, void *arg, bool *last, * notice we unlink'd the filter so we can't get it * back in the fastmap. */ - route4_reset_fastmap(head); + route4_reset_fastmap(head, f); /* Delete it */ tcf_unbind_filter(tp, &f->res);@@ -558,7 +563,7 @@ static int route4_change(struct net *net, struct sk_buff *in_skb, } } - route4_reset_fastmap(head); + route4_reset_fastmap(head, fold); *arg = f; if (fold) { tcf_unbind_filter(tp, &fold->res);