net_sched: fix ops->bind_class() implementations

[ Upstream commit 2e24cd755552350b94a7617617c6877b8cbcb701 ]

The current implementations of ops->bind_class() are merely
searching for classid and updating class in the struct tcf_result,
without invoking either of cl_ops->bind_tcf() or
cl_ops->unbind_tcf(). This breaks the design of them as qdisc's
like cbq use them to count filters too. This is why syzbot triggered
the warning in cbq_destroy_class().

In order to fix this, we have to call cl_ops->bind_tcf() and
cl_ops->unbind_tcf() like the filter binding path. This patch does
so by refactoring out two helper functions __tcf_bind_filter()
and __tcf_unbind_filter(), which are lockless and accept a Qdisc
pointer, then teaching each implementation to call them correctly.

Note, we merely pass the Qdisc pointer as an opaque pointer to
each filter, they only need to pass it down to the helper
functions without understanding it at all.

Fixes: 07d79fc7d94e ("net_sched: add reverse binding for tc class")
Reported-and-tested-by: syzbot+0a0596220218fcb603a8@syzkaller.appspotmail.com
Reported-and-tested-by: syzbot+63bdb6006961d8c917c6@syzkaller.appspotmail.com
Cc: Jamal Hadi Salim <jhs@mojatatu.com>
Cc: Jiri Pirko <jiri@resnulli.us>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h
index 75a3f3fdb3591720d266fd33d98c608fc673b6ca..c1162f2fde783df6937dadaf1205c357c56e3433 100644 (file)
--- a/include/net/pkt_cls.h
+++ b/include/net/pkt_cls.h
@@ -206,31 +206,38 @@ __cls_set_class(unsigned long *clp, unsigned long cl)
        return xchg(clp, cl);
 }
 
-static inline unsigned long
-cls_set_class(struct Qdisc *q, unsigned long *clp, unsigned long cl)
+static inline void
+__tcf_bind_filter(struct Qdisc *q, struct tcf_result *r, unsigned long base)
 {
-       unsigned long old_cl;
+       unsigned long cl;
 
-       sch_tree_lock(q);
-       old_cl = __cls_set_class(clp, cl);
-       sch_tree_unlock(q);
-       return old_cl;
+       cl = q->ops->cl_ops->bind_tcf(q, base, r->classid);
+       cl = __cls_set_class(&r->class, cl);
+       if (cl)
+               q->ops->cl_ops->unbind_tcf(q, cl);
 }
 
 static inline void
 tcf_bind_filter(struct tcf_proto *tp, struct tcf_result *r, unsigned long base)
 {
        struct Qdisc *q = tp->chain->block->q;
-       unsigned long cl;
 
        /* Check q as it is not set for shared blocks. In that case,
         * setting class is not supported.
         */
        if (!q)
                return;
-       cl = q->ops->cl_ops->bind_tcf(q, base, r->classid);
-       cl = cls_set_class(q, &r->class, cl);
-       if (cl)
+       sch_tree_lock(q);
+       __tcf_bind_filter(q, r, base);
+       sch_tree_unlock(q);
+}
+
+static inline void
+__tcf_unbind_filter(struct Qdisc *q, struct tcf_result *r)
+{
+       unsigned long cl;
+
+       if ((cl = __cls_set_class(&r->class, 0)) != 0)
                q->ops->cl_ops->unbind_tcf(q, cl);
 }
 
@@ -238,12 +245,10 @@ static inline void
 tcf_unbind_filter(struct tcf_proto *tp, struct tcf_result *r)
 {
        struct Qdisc *q = tp->chain->block->q;
-       unsigned long cl;
 
        if (!q)
                return;
-       if ((cl = __cls_set_class(&r->class, 0)) != 0)
-               q->ops->cl_ops->unbind_tcf(q, cl);
+       __tcf_unbind_filter(q, r);
 }
 
 struct tcf_exts {

diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h
index c9cd5086bd544e1be75b594fa34d8c5113f6526e..d737a6a2600be802672bdcd119a782aad1a5ec9d 100644 (file)
--- a/include/net/sch_generic.h
+++ b/include/net/sch_generic.h
@@ -273,7 +273,8 @@ struct tcf_proto_ops {
        int                     (*reoffload)(struct tcf_proto *tp, bool add,
                                             tc_setup_cb_t *cb, void *cb_priv,
                                             struct netlink_ext_ack *extack);
-       void                    (*bind_class)(void *, u32, unsigned long);
+       void                    (*bind_class)(void *, u32, unsigned long,
+                                             void *, unsigned long);
        void *                  (*tmplt_create)(struct net *net,
                                                struct tcf_chain *chain,
                                                struct nlattr **tca,

diff --git a/net/sched/cls_basic.c b/net/sched/cls_basic.c
index 6a5dce8baf190109f2e9e6902f872236211fc3ff..14098da696f2a61d52d4fb7a75b6d8e7e9384be9 100644 (file)
--- a/net/sched/cls_basic.c
+++ b/net/sched/cls_basic.c
@@ -254,12 +254,17 @@ static void basic_walk(struct tcf_proto *tp, struct tcf_walker *arg)
        }
 }
 
-static void basic_bind_class(void *fh, u32 classid, unsigned long cl)
+static void basic_bind_class(void *fh, u32 classid, unsigned long cl, void *q,
+                            unsigned long base)
 {
        struct basic_filter *f = fh;
 
-       if (f && f->res.classid == classid)
-               f->res.class = cl;
+       if (f && f->res.classid == classid) {
+               if (cl)
+                       __tcf_bind_filter(q, &f->res, base);
+               else
+                       __tcf_unbind_filter(q, &f->res);
+       }
 }
 
 static int basic_dump(struct net *net, struct tcf_proto *tp, void *fh,

diff --git a/net/sched/cls_bpf.c b/net/sched/cls_bpf.c
index fa6fe2fe0f32b521ccb7a77f4e19693e08baf859..5d100126cbf3ea830b0fba200bcf1c86297018e8 100644 (file)
--- a/net/sched/cls_bpf.c
+++ b/net/sched/cls_bpf.c
@@ -627,12 +627,17 @@ static int cls_bpf_dump(struct net *net, struct tcf_proto *tp, void *fh,
        return -1;
 }
 
-static void cls_bpf_bind_class(void *fh, u32 classid, unsigned long cl)
+static void cls_bpf_bind_class(void *fh, u32 classid, unsigned long cl,
+                              void *q, unsigned long base)
 {
        struct cls_bpf_prog *prog = fh;
 
-       if (prog && prog->res.classid == classid)
-               prog->res.class = cl;
+       if (prog && prog->res.classid == classid) {
+               if (cl)
+                       __tcf_bind_filter(q, &prog->res, base);
+               else
+                       __tcf_unbind_filter(q, &prog->res);
+       }
 }
 
 static void cls_bpf_walk(struct tcf_proto *tp, struct tcf_walker *arg)

diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
index 09b359784629e46b1ce5f60c42e193196f312549..22415311f3246f5f264df31f17d8a5f9e266b324 100644 (file)
--- a/net/sched/cls_flower.c
+++ b/net/sched/cls_flower.c
@@ -1942,12 +1942,17 @@ static int fl_tmplt_dump(struct sk_buff *skb, struct net *net, void *tmplt_priv)
        return -EMSGSIZE;
 }
 
-static void fl_bind_class(void *fh, u32 classid, unsigned long cl)
+static void fl_bind_class(void *fh, u32 classid, unsigned long cl, void *q,
+                         unsigned long base)
 {
        struct cls_fl_filter *f = fh;
 
-       if (f && f->res.classid == classid)
-               f->res.class = cl;
+       if (f && f->res.classid == classid) {
+               if (cl)
+                       __tcf_bind_filter(q, &f->res, base);
+               else
+                       __tcf_unbind_filter(q, &f->res);
+       }
 }
 
 static struct tcf_proto_ops cls_fl_ops __read_mostly = {

diff --git a/net/sched/cls_fw.c b/net/sched/cls_fw.c
index 29eeeaf3ea4401ea43496f4a9da8916fa744a2ef..cb2c62605fc764dc3cae9eb4c87ed6b56b03e9db 100644 (file)
--- a/net/sched/cls_fw.c
+++ b/net/sched/cls_fw.c
@@ -432,12 +432,17 @@ static int fw_dump(struct net *net, struct tcf_proto *tp, void *fh,
        return -1;
 }
 
-static void fw_bind_class(void *fh, u32 classid, unsigned long cl)
+static void fw_bind_class(void *fh, u32 classid, unsigned long cl, void *q,
+                         unsigned long base)
 {
        struct fw_filter *f = fh;
 
-       if (f && f->res.classid == classid)
-               f->res.class = cl;
+       if (f && f->res.classid == classid) {
+               if (cl)
+                       __tcf_bind_filter(q, &f->res, base);
+               else
+                       __tcf_unbind_filter(q, &f->res);
+       }
 }
 
 static struct tcf_proto_ops cls_fw_ops __read_mostly = {

diff --git a/net/sched/cls_matchall.c b/net/sched/cls_matchall.c
index 621bc1d5b0579043f9f632a90e6b0ae9d3c3dedb..40be745db357a4d4d294447c82d5423363eb1eba 100644 (file)
--- a/net/sched/cls_matchall.c
+++ b/net/sched/cls_matchall.c
@@ -310,12 +310,17 @@ static int mall_dump(struct net *net, struct tcf_proto *tp, void *fh,
        return -1;
 }
 
-static void mall_bind_class(void *fh, u32 classid, unsigned long cl)
+static void mall_bind_class(void *fh, u32 classid, unsigned long cl, void *q,
+                           unsigned long base)
 {
        struct cls_mall_head *head = fh;
 
-       if (head && head->res.classid == classid)
-               head->res.class = cl;
+       if (head && head->res.classid == classid) {
+               if (cl)
+                       __tcf_bind_filter(q, &head->res, base);
+               else
+                       __tcf_unbind_filter(q, &head->res);
+       }
 }
 
 static struct tcf_proto_ops cls_mall_ops __read_mostly = {

diff --git a/net/sched/cls_route.c b/net/sched/cls_route.c
index 0404aa5fa7cbb91a008b75a5ce3ff2f1b563d00e..37ae23db4a440844c24489acaf7fda64ca65b5c0 100644 (file)
--- a/net/sched/cls_route.c
+++ b/net/sched/cls_route.c
@@ -645,12 +645,17 @@ static int route4_dump(struct net *net, struct tcf_proto *tp, void *fh,
        return -1;
 }
 
-static void route4_bind_class(void *fh, u32 classid, unsigned long cl)
+static void route4_bind_class(void *fh, u32 classid, unsigned long cl, void *q,
+                             unsigned long base)
 {
        struct route4_filter *f = fh;
 
-       if (f && f->res.classid == classid)
-               f->res.class = cl;
+       if (f && f->res.classid == classid) {
+               if (cl)
+                       __tcf_bind_filter(q, &f->res, base);
+               else
+                       __tcf_unbind_filter(q, &f->res);
+       }
 }
 
 static struct tcf_proto_ops cls_route4_ops __read_mostly = {

diff --git a/net/sched/cls_rsvp.h b/net/sched/cls_rsvp.h
index e9ccf7daea7d84eeefe5e872b3497a6f0edc7ea9..6d30a291bcd26813842d403ed52cc5545e0e563d 100644 (file)
--- a/net/sched/cls_rsvp.h
+++ b/net/sched/cls_rsvp.h
@@ -736,12 +736,17 @@ static int rsvp_dump(struct net *net, struct tcf_proto *tp, void *fh,
        return -1;
 }
 
-static void rsvp_bind_class(void *fh, u32 classid, unsigned long cl)
+static void rsvp_bind_class(void *fh, u32 classid, unsigned long cl, void *q,
+                           unsigned long base)
 {
        struct rsvp_filter *f = fh;
 
-       if (f && f->res.classid == classid)
-               f->res.class = cl;
+       if (f && f->res.classid == classid) {
+               if (cl)
+                       __tcf_bind_filter(q, &f->res, base);
+               else
+                       __tcf_unbind_filter(q, &f->res);
+       }
 }
 
 static struct tcf_proto_ops RSVP_OPS __read_mostly = {

diff --git a/net/sched/cls_tcindex.c b/net/sched/cls_tcindex.c
index 38bb882bb9587e57195f37ac81b3da8bdec81cdd..edf27365f91c315e9be8e570b526a013ec26ff57 100644 (file)
--- a/net/sched/cls_tcindex.c
+++ b/net/sched/cls_tcindex.c
@@ -652,12 +652,17 @@ static int tcindex_dump(struct net *net, struct tcf_proto *tp, void *fh,
        return -1;
 }
 
-static void tcindex_bind_class(void *fh, u32 classid, unsigned long cl)
+static void tcindex_bind_class(void *fh, u32 classid, unsigned long cl,
+                              void *q, unsigned long base)
 {
        struct tcindex_filter_result *r = fh;
 
-       if (r && r->res.classid == classid)
-               r->res.class = cl;
+       if (r && r->res.classid == classid) {
+               if (cl)
+                       __tcf_bind_filter(q, &r->res, base);
+               else
+                       __tcf_unbind_filter(q, &r->res);
+       }
 }
 
 static struct tcf_proto_ops cls_tcindex_ops __read_mostly = {

diff --git a/net/sched/cls_u32.c b/net/sched/cls_u32.c
index b2c3406a2cf292d93aa09280e19a5e149fc0673f..fe246e03fcd9c4134f642cda6a559289028022e5 100644 (file)
--- a/net/sched/cls_u32.c
+++ b/net/sched/cls_u32.c
@@ -1315,12 +1315,17 @@ static int u32_reoffload(struct tcf_proto *tp, bool add, tc_setup_cb_t *cb,
        return 0;
 }
 
-static void u32_bind_class(void *fh, u32 classid, unsigned long cl)
+static void u32_bind_class(void *fh, u32 classid, unsigned long cl, void *q,
+                          unsigned long base)
 {
        struct tc_u_knode *n = fh;
 
-       if (n && n->res.classid == classid)
-               n->res.class = cl;
+       if (n && n->res.classid == classid) {
+               if (cl)
+                       __tcf_bind_filter(q, &n->res, base);
+               else
+                       __tcf_unbind_filter(q, &n->res);
+       }
 }
 
 static int u32_dump(struct net *net, struct tcf_proto *tp, void *fh,

diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c
index 84fdc4857771bf62fc66f4c858f75b58db95de7a..39e319d04bb87b5ee98d5ec0935c13f76ace3f02 100644 (file)
--- a/net/sched/sch_api.c
+++ b/net/sched/sch_api.c
@@ -1803,8 +1803,9 @@ static int tclass_del_notify(struct net *net,
 
 struct tcf_bind_args {
        struct tcf_walker w;
-       u32 classid;
+       unsigned long base;
        unsigned long cl;
+       u32 classid;
 };
 
 static int tcf_node_bind(struct tcf_proto *tp, void *n, struct tcf_walker *arg)
@@ -1815,7 +1816,7 @@ static int tcf_node_bind(struct tcf_proto *tp, void *n, struct tcf_walker *arg)
                struct Qdisc *q = tcf_block_q(tp->chain->block);
 
                sch_tree_lock(q);
-               tp->ops->bind_class(n, a->classid, a->cl);
+               tp->ops->bind_class(n, a->classid, a->cl, q, a->base);
                sch_tree_unlock(q);
        }
        return 0;
@@ -1846,6 +1847,7 @@ static void tc_bind_tclass(struct Qdisc *q, u32 portid, u32 clid,
 
                        arg.w.fn = tcf_node_bind;
                        arg.classid = clid;
+                       arg.base = cl;
                        arg.cl = new_cl;
                        tp->ops->walk(tp, &arg.w);
                }