Re: [PATCH net 1/1] net/sched: cls_route: fix fastmap use-after-free on filter

On 7/28/26 2:37 PM, Jamal Hadi Salim wrote:

On Tue, Jul 28, 2026 at 6:34 AM Paolo Abeni [off-list ref] wrote:

quoted

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?

Usually the tool properly updates the old revision status, but sometimes
it fails do to that.

Double checking that the old revision is marked as superseded in PW
after posting the new one could help.

quoted

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.

Given the amount of effort involved in rtnl lock contention reduction, I
think it would be great if we could avoid the synchronize_rcu(), if it
does not block the fix for an unreasonable amount of time.

quoted

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.

I would appreciate that, thanks!

Paolo