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

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

[Jamal Hadi Salim]

A case of partially rcu'ed structs.
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) deletes
the last filter on a tp. Both triggered KASAN slab-use-after-free reports
in the route4 fastmap paths.

Fix:
Move the fastmap flush to the writer side under RTNL, ordered after
synchronize_rcu(). This ensures stale fastmap entries written by in-flight
readers are visible (readers have finished) and no new readers can find the
unlinked filter, so the subsequent route4_reset_fastmap(head) flushes them
safely while head is valid under RTNL.

Note: This fix will slow down deletes and filter add/replace, since
synchronize_rcu() is called under RTNL in route4_delete() and
route4_change(). An alternative fix would have been to introduce refcnt
to the head struct but that is a much bigger patch. We justify this as
a reasonable fix.

Fixes: 1109c00547fc ("net: sched: RCU cls_route")
Reported-by: zdi-disclosures@trendmicro.com
Reported-by: Santosh Kalluri <santosh.kalluri129@gmail.com>
Suggested-by: Santosh Kalluri <santosh.kalluri129@gmail.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>
---
 net/sched/cls_route.c | 42 +++++++++++++++++++++++++++++++++++-------
 1 file changed, 35 insertions(+), 7 deletions(-)

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);
 				}
 			}
 			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);
+
 	kfree_rcu(head, rcu);
 }
 
@@ -334,10 +347,16 @@ 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.
+			/* This code path assumes we have the RTNL lock.
+			 * We wait for in-flight readers that may still hold
+			 * the filter pointer and publish it into the fastmap
+			 * after we unlinked it.
+			 * Once done, no new reader can find the filter on the
+			 * chain, so the only stale fastmap entries are the
+			 * ones those readers just wrote. Flush them now
+			 * while head is still valid.
 			 */
+			synchronize_rcu();
 			route4_reset_fastmap(head);
 
 			/* Delete it */
@@ -558,6 +577,15 @@ static int route4_change(struct net *net, struct sk_buff *in_skb,
 		}
 	}
 
+	/* Assuming RTNL lock. Wait for in-flight readers that may still
+	 * hold a filter pointer and publish it into the fastmap after we
+	 * inserted the new filter (or unlinked fold when replacing).
+	 * Once they are done, flush the stale entries while head is still
+	 * valid.  Needed on both the creation and replace paths: on
+	 * creation, readers may have cached a ROUTE4_FAILURE entry for the
+	 * (id, iif) tuple that the new filter now matches.
+	 */
+	synchronize_rcu();
 	route4_reset_fastmap(head);
 	*arg = f;
 	if (fold) {
-- 
2.34.1