net_sched: fix ops->bind_class() implementations

BugLink: https://bugs.launchpad.net/bugs/1864904
[ 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>
Signed-off-by: Kamal Mostafa <kamal@canonical.com>
Signed-off-by: Khalid Elmously <khalid.elmously@canonical.com>

diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h
index d076a0b7af8f77402d99f45924b103111ddbdd4b..dfedcf5b8e477494c349d32ce7fad88b6db8ef42 100644 (file)
--- a/include/net/pkt_cls.h
+++ b/include/net/pkt_cls.h
@@ -186,31 +186,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);
 }
 
@@ -218,12 +225,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 ab1dfa9c829c615c43dcf0efe9f0cba22345ef0d..5ada472fc8f97c932d28be99b534909a466079a0 100644 (file)
--- a/include/net/sch_generic.h
+++ b/include/net/sch_generic.h
@@ -227,7 +227,8 @@ struct tcf_proto_ops {
                                        void **, bool);
        int                     (*delete)(struct tcf_proto*, void *, bool*);
        void                    (*walk)(struct tcf_proto*, struct tcf_walker *arg);
-       void                    (*bind_class)(void *, u32, unsigned long);
+       void                    (*bind_class)(void *, u32, unsigned long,
+                                             void *, unsigned long);
 
        /* rtnetlink specific */
        int                     (*dump)(struct net*, struct tcf_proto*, void *,

diff --git a/net/sched/cls_basic.c b/net/sched/cls_basic.c
index 9afb739329830c7b970ecb51a95c42b7483be18a..2e51d1169eff73aea6811566ad7a98f63416b5c1 100644 (file)
--- a/net/sched/cls_basic.c
+++ b/net/sched/cls_basic.c
@@ -255,12 +255,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 d98f4e24f0df565b5ecf2b9edc2145199944b1f2..0834573fcf483c3b55c13ec21778ede5f15c9b8b 100644 (file)
--- a/net/sched/cls_bpf.c
+++ b/net/sched/cls_bpf.c
@@ -621,12 +621,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 26c91780c46b2c372a8613db14e7191f2af340d9..9fa562d69cbbb3bb855c4fa0329153181e60e521 100644 (file)
--- a/net/sched/cls_flower.c
+++ b/net/sched/cls_flower.c
@@ -1323,12 +1323,17 @@ nla_put_failure:
        return -1;
 }
 
-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 e2d476cc24f75821abd6869841dcdec8d7180b91..564815057730cb5bc413afc5cc9092d3da6c3eb4 100644 (file)
--- a/net/sched/cls_fw.c
+++ b/net/sched/cls_fw.c
@@ -430,12 +430,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 40b81f6faf97c578fc3483ec602396c44f0cb038..f317a7507f0d4d660dd3a1a4535ccc9c128a0006 100644 (file)
--- a/net/sched/cls_matchall.c
+++ b/net/sched/cls_matchall.c
@@ -272,12 +272,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 9b1eecd1137aed1ef5260fbad947197d1aead8f3..19ebe42f9981e35b31f0af2bdc5435e1c2aa695f 100644 (file)
--- a/net/sched/cls_route.c
+++ b/net/sched/cls_route.c
@@ -643,12 +643,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 1b6b39a44e81cce99a4bf1d778c63e5dbbf21064..d1484f86ad63dcbe17d64e0a3783cec3bec4b255 100644 (file)
--- a/net/sched/cls_rsvp.h
+++ b/net/sched/cls_rsvp.h
@@ -736,12 +736,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 d64ccc959250d4a7a86462a924a2861f9adfdfe8..27e43f2b3a7bee3545e75b20639d3db4e4e32fea 100644 (file)
--- a/net/sched/cls_tcindex.c
+++ b/net/sched/cls_tcindex.c
@@ -648,12 +648,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 04a34f6155247be7c0e0a82e12fb2a628e404433..34d5f91f5b021b43d2fd5f06965a7f0d209dcadc 100644 (file)
--- a/net/sched/cls_u32.c
+++ b/net/sched/cls_u32.c
@@ -1151,12 +1151,17 @@ static void u32_walk(struct tcf_proto *tp, struct tcf_walker *arg)
        }
 }
 
-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 e921f283aecde9343967fdbfebffd97c7650fb35..17100d91bdaabe5f53db02d8a06c9a95624f97bb 100644 (file)
--- a/net/sched/sch_api.c
+++ b/net/sched/sch_api.c
@@ -1656,8 +1656,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)
@@ -1668,7 +1669,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;
@@ -1699,6 +1700,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);
                }