Netdev List
 help / color / mirror / Atom feed
* [PATCH net] net/sched: cls_route: fix fastmap use-after-free on filter
@ 2026-07-29  9:44 Jamal Hadi Salim
  2026-07-29  9:47 ` Jamal Hadi Salim
  0 siblings, 1 reply; 2+ messages in thread
From: Jamal Hadi Salim @ 2026-07-29  9:44 UTC (permalink / raw)
  To: netdev
  Cc: pabeni, kuba, davem, edumazet, horms, john.fastabend, jiri,
	zdi-disclosures, santosh.kalluri129, victor, security, stable,
	Jamal Hadi Salim

The route4 classifier maintains a 16-slot fastmap cache that stores raw
struct route4_filter pointers indexed by (id, iif). The reader
(route4_classify) populates this cache via route4_set_fastmap() for every
classified packet that hits a filter. The writer (route4_delete,
route4_change) clears the cache via route4_reset_fastmap() before
RCU-deferred kfree of the filter.

This creates a UAF race:
 1. Reader walks the RCU-protected bucket chain, finds filter f
 2. Writer unlinks f, calls route4_reset_fastmap(), then tcf_queue_work()
 3. Reader calls route4_set_fastmap() and writes f into the cache
    *after* the writer's reset, caching a pointer about to be freed
 4. After the RCU grace period, kfree(f) executes
 5. Next classified packet on the same (id, iif) tuple hits the stale
    fastmap entry and reads f->res from freed memory

Reproduced with an mdelay(100) accelerator in route4_set_fastmap() and a
concurrent add/delete stress test (provided by both zdi and Santosh).
Both triggered KASAN slab-use-after-free reports in the route4 fastmap
paths.

Fix:
Introduce a per-filter boolean dying flag to suppress stale fastmap
republishing by in-flight readers.

Fixes: 1109c00547fc ("net: sched: RCU cls_route")
Reported-by: zdi-disclosures@trendmicro.com
Reported-by: Santosh Kalluri <santosh.kalluri129@gmail.com>
Suggested-by: Paolo Abeni <pabeni@redhat.com>
Tested-by: Victor Nogueira <victor@mojatatu.com>
Tested-by: Santosh Kalluri <santosh.kalluri129@gmail.com>
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>

---
V1->V2
1. Fix based on feedback from the Sashikos
https://sashiko.dev/#/patchset/20260723105210.817079-1-jhs%40mojatatu.com
https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260723105210.817079-1-jhs%40mojatatu.com

Restore the "if (tcf_exts_get_net(&f->exts)) route4_queue_work(f);
else __route4_delete_filter(f);"
preserving the tcf_exts_get_net() contract for netns teardown.

V2->V3
Revert earlier suggestion from Santosh's rcu sync to Paolo's suggestion to use
the dying flag approach.
---
 net/sched/cls_route.c | 35 ++++++++++++++++++++++++++---------
 1 file changed, 26 insertions(+), 9 deletions(-)

diff --git a/net/sched/cls_route.c b/net/sched/cls_route.c
index bd6f945bd388..eded7aacd3f7 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 == ROUTE4_FAILURE || !f->dying) {
+		head->fastmap[h].id = id;
+		head->fastmap[h].iif = iif;
+		head->fastmap[h].filter = f;
+	}
 	spin_unlock_bh(&fastmap_lock);
 }
 
@@ -297,6 +302,13 @@ 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);
+					/* Mark the filter dying under fastmap_lock so
+					 * any in-flight reader that still holds it
+					 * will skip the republish in route4_set_fastmap().
+					 */
+					spin_lock_bh(&fastmap_lock);
+					f->dying = true;
+					spin_unlock_bh(&fastmap_lock);
 					if (tcf_exts_get_net(&f->exts))
 						route4_queue_work(f);
 					else
@@ -307,6 +319,11 @@ static void route4_destroy(struct tcf_proto *tp, bool rtnl_held,
 			kfree_rcu(b, rcu);
 		}
 	}
+
+	/* All filters are unlinked and marked dying, so no in-flight
+	 * reader can republish a stale entry after this reset.
+	 */
+	route4_reset_fastmap(head, NULL);
 	kfree_rcu(head, rcu);
 }
 
@@ -334,11 +351,11 @@ static int route4_delete(struct tcf_proto *tp, void *arg, bool *last,
 			/* unlink it */
 			RCU_INIT_POINTER(*fp, rtnl_dereference(f->next));
 
-			/* Remove any fastmap lookups that might ref filter
-			 * notice we unlink'd the filter so we can't get it
-			 * back in the fastmap.
+			/* Clear any fastmap entries that may ref this filter and
+			 * mark it dying so in-flight readers can't republish it
+			 * after the reset.
 			 */
-			route4_reset_fastmap(head);
+			route4_reset_fastmap(head, f);
 
 			/* Delete it */
 			tcf_unbind_filter(tp, &f->res);
@@ -558,7 +575,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);
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH net] net/sched: cls_route: fix fastmap use-after-free on filter
  2026-07-29  9:44 [PATCH net] net/sched: cls_route: fix fastmap use-after-free on filter Jamal Hadi Salim
@ 2026-07-29  9:47 ` Jamal Hadi Salim
  0 siblings, 0 replies; 2+ messages in thread
From: Jamal Hadi Salim @ 2026-07-29  9:47 UTC (permalink / raw)
  To: netdev
  Cc: pabeni, kuba, davem, edumazet, horms, john.fastabend, jiri,
	zdi-disclosures, santosh.kalluri129, victor, security, stable

On Wed, Jul 29, 2026 at 5:44 AM Jamal Hadi Salim <jhs@mojatatu.com> wrote:
>
> The route4 classifier maintains a 16-slot fastmap cache that stores raw
> struct route4_filter pointers indexed by (id, iif). The reader
> (route4_classify) populates this cache via route4_set_fastmap() for every
> classified packet that hits a filter. The writer (route4_delete,
> route4_change) clears the cache via route4_reset_fastmap() before
> RCU-deferred kfree of the filter.
>

Sigh. I forgot to add v3 on the subject. I could resend if needed.


cheers,
jamal

> This creates a UAF race:
>  1. Reader walks the RCU-protected bucket chain, finds filter f
>  2. Writer unlinks f, calls route4_reset_fastmap(), then tcf_queue_work()
>  3. Reader calls route4_set_fastmap() and writes f into the cache
>     *after* the writer's reset, caching a pointer about to be freed
>  4. After the RCU grace period, kfree(f) executes
>  5. Next classified packet on the same (id, iif) tuple hits the stale
>     fastmap entry and reads f->res from freed memory
>
> Reproduced with an mdelay(100) accelerator in route4_set_fastmap() and a
> concurrent add/delete stress test (provided by both zdi and Santosh).
> Both triggered KASAN slab-use-after-free reports in the route4 fastmap
> paths.
>
> Fix:
> Introduce a per-filter boolean dying flag to suppress stale fastmap
> republishing by in-flight readers.
>
> Fixes: 1109c00547fc ("net: sched: RCU cls_route")
> Reported-by: zdi-disclosures@trendmicro.com
> Reported-by: Santosh Kalluri <santosh.kalluri129@gmail.com>
> Suggested-by: Paolo Abeni <pabeni@redhat.com>
> Tested-by: Victor Nogueira <victor@mojatatu.com>
> Tested-by: Santosh Kalluri <santosh.kalluri129@gmail.com>
> Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
>
> ---
> V1->V2
> 1. Fix based on feedback from the Sashikos
> https://sashiko.dev/#/patchset/20260723105210.817079-1-jhs%40mojatatu.com
> https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260723105210.817079-1-jhs%40mojatatu.com
>
> Restore the "if (tcf_exts_get_net(&f->exts)) route4_queue_work(f);
> else __route4_delete_filter(f);"
> preserving the tcf_exts_get_net() contract for netns teardown.
>
> V2->V3
> Revert earlier suggestion from Santosh's rcu sync to Paolo's suggestion to use
> the dying flag approach.
> ---
>  net/sched/cls_route.c | 35 ++++++++++++++++++++++++++---------
>  1 file changed, 26 insertions(+), 9 deletions(-)
>
> diff --git a/net/sched/cls_route.c b/net/sched/cls_route.c
> index bd6f945bd388..eded7aacd3f7 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 == ROUTE4_FAILURE || !f->dying) {
> +               head->fastmap[h].id = id;
> +               head->fastmap[h].iif = iif;
> +               head->fastmap[h].filter = f;
> +       }
>         spin_unlock_bh(&fastmap_lock);
>  }
>
> @@ -297,6 +302,13 @@ 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);
> +                                       /* Mark the filter dying under fastmap_lock so
> +                                        * any in-flight reader that still holds it
> +                                        * will skip the republish in route4_set_fastmap().
> +                                        */
> +                                       spin_lock_bh(&fastmap_lock);
> +                                       f->dying = true;
> +                                       spin_unlock_bh(&fastmap_lock);
>                                         if (tcf_exts_get_net(&f->exts))
>                                                 route4_queue_work(f);
>                                         else
> @@ -307,6 +319,11 @@ static void route4_destroy(struct tcf_proto *tp, bool rtnl_held,
>                         kfree_rcu(b, rcu);
>                 }
>         }
> +
> +       /* All filters are unlinked and marked dying, so no in-flight
> +        * reader can republish a stale entry after this reset.
> +        */
> +       route4_reset_fastmap(head, NULL);
>         kfree_rcu(head, rcu);
>  }
>
> @@ -334,11 +351,11 @@ static int route4_delete(struct tcf_proto *tp, void *arg, bool *last,
>                         /* unlink it */
>                         RCU_INIT_POINTER(*fp, rtnl_dereference(f->next));
>
> -                       /* Remove any fastmap lookups that might ref filter
> -                        * notice we unlink'd the filter so we can't get it
> -                        * back in the fastmap.
> +                       /* Clear any fastmap entries that may ref this filter and
> +                        * mark it dying so in-flight readers can't republish it
> +                        * after the reset.
>                          */
> -                       route4_reset_fastmap(head);
> +                       route4_reset_fastmap(head, f);
>
>                         /* Delete it */
>                         tcf_unbind_filter(tp, &f->res);
> @@ -558,7 +575,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);
> --
> 2.34.1
>

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2026-07-29  9:47 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-29  9:44 [PATCH net] net/sched: cls_route: fix fastmap use-after-free on filter Jamal Hadi Salim
2026-07-29  9:47 ` Jamal Hadi Salim

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox