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

[Paolo Abeni <pabeni@redhat.com>]

On 7/23/26 12:52 PM, Jamal Hadi Salim wrote:
> 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



>  				}
>  			}
>  			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. 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!!!):

---
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);