net_sched: fix ops->bind_class() implementations

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>

diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h
index ce036492986afab629ac0ccde8b2c7c26da3391a..a972244ab1931a2fd52a41efa4f7a6cd78d8746a 100644 (file)
--- a/include/net/pkt_cls.h
+++ b/include/net/pkt_cls.h
@@ -141,31 +141,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);
 }
 
@@ -173,12 +180,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 fceddf89592af4483431d012873bc611a80d7d50..151208704ed2e6d71c6a06afcd1473e68428c88b 100644 (file)
--- a/include/net/sch_generic.h
+++ b/include/net/sch_generic.h
@@ -318,7 +318,8 @@ struct tcf_proto_ops {
                                          void *type_data);
        void                    (*hw_del)(struct tcf_proto *tp,
                                          void *type_data);
-       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 4aafbe3d435c68600f8fa51a3fdd1fe901b8d638..f256a7c69093ab610cb144f485e3b48667f4614b 100644 (file)
--- a/net/sched/cls_basic.c
+++ b/net/sched/cls_basic.c
@@ -263,12 +263,17 @@ skip:
        }
 }
 
-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 8229ed4a67bee6eaa46cd365f202b6e698b1d7a9..6e3e63db0e0168d8543602632b3bdeecaa908c35 100644 (file)
--- a/net/sched/cls_bpf.c
+++ b/net/sched/cls_bpf.c
@@ -631,12 +631,17 @@ nla_put_failure:
        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 b0f42e62dd7607b4ee55ad39443ca075b9356bcf..f9c0d1e8d380152e4900e78e98c3edfca5deda29 100644 (file)
--- a/net/sched/cls_flower.c
+++ b/net/sched/cls_flower.c
@@ -2765,12 +2765,17 @@ nla_put_failure:
        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 bool fl_delete_empty(struct tcf_proto *tp)

diff --git a/net/sched/cls_fw.c b/net/sched/cls_fw.c
index c9496c920d6fac544e8fbbe4a40965b47b4d7aad..ec945294626a8b3d1cd761b6a2636290fc47d659 100644 (file)
--- a/net/sched/cls_fw.c
+++ b/net/sched/cls_fw.c
@@ -419,12 +419,17 @@ nla_put_failure:
        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 7fc2eb62aa982651b11ab07f575fdfdaf63a11d3..039cc86974f4583472583c0260031ace609501b2 100644 (file)
--- a/net/sched/cls_matchall.c
+++ b/net/sched/cls_matchall.c
@@ -393,12 +393,17 @@ nla_put_failure:
        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 2d9e0b4484ea7133bc7294f0ca123cd813e952f8..6f8786b06bde17721f17280a58a66baab76a684b 100644 (file)
--- a/net/sched/cls_route.c
+++ b/net/sched/cls_route.c
@@ -641,12 +641,17 @@ nla_put_failure:
        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 2f3c03b25d5d7a8e44460276fc52a67ae204693e..c22624131949746e5f0f25654a5efce7466d9727 100644 (file)
--- a/net/sched/cls_rsvp.h
+++ b/net/sched/cls_rsvp.h
@@ -738,12 +738,17 @@ nla_put_failure:
        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 e573e5a5c7945cd9109d95949bef86802edda49c..3d4a1280352f35f404ef88cf73491552b7feffdd 100644 (file)
--- a/net/sched/cls_tcindex.c
+++ b/net/sched/cls_tcindex.c
@@ -654,12 +654,17 @@ nla_put_failure:
        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 a0e6fac613de215bd2ca588624e2f3edda7c19ff..e15ff335953deef36e55901d8f4ce34e15ed676f 100644 (file)
--- a/net/sched/cls_u32.c
+++ b/net/sched/cls_u32.c
@@ -1255,12 +1255,17 @@ static int u32_reoffload(struct tcf_proto *tp, bool add, flow_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 1047825d9f48d546fe1339a25f3f86979e7ac801..943ad34253804a16ea29128f3981996fab40d8e4 100644 (file)
--- a/net/sched/sch_api.c
+++ b/net/sched/sch_api.c
@@ -1891,8 +1891,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)
@@ -1903,7 +1904,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;
@@ -1936,6 +1937,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, true);
                }