| From 1c1d96d97a8b26247fc3626b648b7dd8fc63d494 Mon Sep 17 00:00:00 2001 |
| From: Cong Wang <xiyou.wangcong@gmail.com> |
| Date: Thu, 23 Jan 2020 16:26:18 -0800 |
| Subject: [PATCH] net_sched: fix ops->bind_class() implementations |
| |
| commit 2e24cd755552350b94a7617617c6877b8cbcb701 upstream. |
| |
| 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: Paul Gortmaker <paul.gortmaker@windriver.com> |
| |
| diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h |
| index 514e3c80ecc1..bc692c7c893d 100644 |
| --- a/include/net/pkt_cls.h |
| +++ b/include/net/pkt_cls.h |
| @@ -241,31 +241,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); |
| } |
| |
| @@ -273,12 +280,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 765a8b313d73..666d2ea202e8 100644 |
| --- a/include/net/sch_generic.h |
| +++ b/include/net/sch_generic.h |
| @@ -316,7 +316,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 4aafbe3d435c..f256a7c69093 100644 |
| --- a/net/sched/cls_basic.c |
| +++ b/net/sched/cls_basic.c |
| @@ -263,12 +263,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 691f71830134..64d8a219bf3c 100644 |
| --- a/net/sched/cls_bpf.c |
| +++ b/net/sched/cls_bpf.c |
| @@ -625,12 +625,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 eedd5786c084..dfd5856916bd 100644 |
| --- a/net/sched/cls_flower.c |
| +++ b/net/sched/cls_flower.c |
| @@ -2402,12 +2402,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 4dab833f66cb..b6c36d60ffae 100644 |
| --- 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 38c0a9f0f296..a75c230c67df 100644 |
| --- a/net/sched/cls_matchall.c |
| +++ b/net/sched/cls_matchall.c |
| @@ -391,12 +391,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 2d9e0b4484ea..6f8786b06bde 100644 |
| --- a/net/sched/cls_route.c |
| +++ b/net/sched/cls_route.c |
| @@ -641,12 +641,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 2f3c03b25d5d..c22624131949 100644 |
| --- a/net/sched/cls_rsvp.h |
| +++ b/net/sched/cls_rsvp.h |
| @@ -738,12 +738,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 e573e5a5c794..3d4a1280352f 100644 |
| --- a/net/sched/cls_tcindex.c |
| +++ b/net/sched/cls_tcindex.c |
| @@ -654,12 +654,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 c7727de5e073..0443ac3ecf1e 100644 |
| --- a/net/sched/cls_u32.c |
| +++ b/net/sched/cls_u32.c |
| @@ -1271,12 +1271,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 1047825d9f48..943ad3425380 100644 |
| --- 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); |
| } |
| -- |
| 2.7.4 |
| |