route4_change can move an existing filter to a different top-level bucket: route4_set_parms recomputes the handle from TCA_ROUTE4_TO/ FROM/IIF, and the handle-mismatch check is gated on the 'new' flag, so for an existing filter the new handle may differ from the old one and land in a different bucket. When this happens, the filter is unlinked from the old bucket, but the bucket itself is never freed once it goes empty. The stale empty bucket remains in head->table[], causing route4_delete to report *last=false even after the last live filter is gone. That pins the empty tcf_proto and causes a leak. Fix this by refcounting the filters linked to a bucket and freeing the bucket when the count drops to zero. The existing scan in route4_delete goes away with it. The count is updated at all sites that link or unlink a filter during add, change and delete, and the bucket is dropped from head->table[] as soon as it reaches zero. Conditions to recreate the bug: CONFIG_NET_CLS_ROUTE4=y, CONFIG_NET_SCH_INGRESS=y, CONFIG_NET_CLS_ACT=y. tc qdisc replace dev lo clsact tc filter add dev lo ingress protocol ip pref 100 route from 1 to 1 tc filter change dev lo ingress protocol ip pref 100 handle 0x10001 \ route from 1 to 2 tc filter del dev lo ingress protocol ip pref 100 handle 0x10002 \ route from 1 to 2 tc filter show dev lo ingress | grep -c 'pref 100 route chain 0 ' Fixes: 1e052be69d04 ("net_sched: destroy proto tp when all filters are gone") Reported-by: Vega Acked-by: Jamal Hadi Salim Signed-off-by: Victor Nogueira --- net/sched/cls_route.c | 45 +++++++++++++++++++++---------------------- 1 file changed, 22 insertions(+), 23 deletions(-) diff --git a/net/sched/cls_route.c b/net/sched/cls_route.c index 0d1324c90583..17b0ebb76662 100644 --- a/net/sched/cls_route.c +++ b/net/sched/cls_route.c @@ -11,6 +11,7 @@ #include #include #include +#include #include #include #include @@ -41,6 +42,7 @@ struct route4_head { struct route4_bucket { /* 16 FROM buckets + 16 IIF buckets + 1 wildcard bucket */ struct route4_filter __rcu *ht[16 + 16 + 1]; + refcount_t filters_ref; struct rcu_head rcu; }; @@ -336,7 +338,7 @@ static int route4_delete(struct tcf_proto *tp, void *arg, bool *last, struct route4_filter *nf; struct route4_bucket *b; unsigned int h = 0; - int i, h1; + int h1; if (!head || !f) return -EINVAL; @@ -362,23 +364,14 @@ static int route4_delete(struct tcf_proto *tp, void *arg, bool *last, tcf_exts_get_net(&f->exts); tcf_queue_work(&f->rwork, route4_delete_filter_work); - /* Strip RTNL protected tree */ - for (i = 0; i <= 32; i++) { - struct route4_filter *rt; - - rt = rtnl_dereference(b->ht[i]); - if (rt) - goto out; + if (refcount_dec_and_test(&b->filters_ref)) { + RCU_INIT_POINTER(head->table[to_hash(h)], NULL); + kfree_rcu(b, rcu); } - - /* OK, session has no flows */ - RCU_INIT_POINTER(head->table[to_hash(h)], NULL); - kfree_rcu(b, rcu); break; } } -out: *last = true; for (h1 = 0; h1 <= 256; h1++) { if (rcu_access_pointer(head->table[h1])) { @@ -459,6 +452,7 @@ static int route4_set_parms(struct net *net, struct tcf_proto *tp, if (b == NULL) return -ENOBUFS; + refcount_set(&b->filters_ref, 1); rcu_assign_pointer(head->table[h1], b); } else { unsigned int h2 = from_hash(nhandle >> 16); @@ -468,6 +462,8 @@ static int route4_set_parms(struct net *net, struct tcf_proto *tp, fp = rtnl_dereference(fp->next)) if (fp->handle == f->handle) return -EEXIST; + + refcount_inc(&b->filters_ref); } if (tb[TCA_ROUTE4_TO]) @@ -500,7 +496,7 @@ static int route4_change(struct net *net, struct sk_buff *in_skb, struct route4_filter *fold, *f1, *pfp, *f = NULL; struct route4_bucket *b; struct nlattr *tb[TCA_ROUTE4_MAX + 1]; - unsigned int h, th; + unsigned int h; int err; bool new = true; @@ -560,17 +556,20 @@ static int route4_change(struct net *net, struct sk_buff *in_skb, rcu_assign_pointer(*fp, f); if (fold) { - th = to_hash(fold->handle); + b = fold->bkt; h = from_hash(fold->handle >> 16); - b = rtnl_dereference(head->table[th]); - if (b) { - fp = &b->ht[h]; - for (pfp = rtnl_dereference(*fp); pfp; - fp = &pfp->next, pfp = rtnl_dereference(*fp)) { - if (pfp == fold) { - rcu_assign_pointer(*fp, fold->next); - break; + fp = &b->ht[h]; + for (pfp = rtnl_dereference(*fp); pfp; + fp = &pfp->next, pfp = rtnl_dereference(*fp)) { + if (pfp == fold) { + rcu_assign_pointer(*fp, fold->next); + if (refcount_dec_and_test(&b->filters_ref)) { + unsigned int th = to_hash(fold->handle); + + RCU_INIT_POINTER(head->table[th], NULL); + kfree_rcu(b, rcu); } + break; } } } -- 2.55.0