net/sched: wrap open coded Qdics class filter counter

The 'filter_cnt' counter is used to control a Qdisc class lifetime.
Each filter referecing this class by its id will eventually
increment/decrement this counter in their respective
'add/update/delete' routines.
As these operations are always serialized under rtnl lock, we don't
need an atomic type like 'refcount_t'.

It also means that we lose the overflow/underflow checks already
present in refcount_t, which are valuable to hunt down bugs
where the unsigned counter wraps around as it aids automated tools
like syzkaller to scream in such situations.

Wrap the open coded increment/decrement into helper functions and
add overflow checks to the operations.

Acked-by: Jamal Hadi Salim <jhs@mojatatu.com>
Signed-off-by: Pedro Tammela <pctammela@mojatatu.com>
Reviewed-by: Simon Horman <horms@kernel.org>
Signed-off-by: Paolo Abeni <pabeni@redhat.com>

diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h
index 15be2d9..f232512 100644 (file)
--- a/include/net/sch_generic.h
+++ b/include/net/sch_generic.h
@@ -599,6 +599,7 @@ get_default_qdisc_ops(const struct net_device *dev, int ntx)
 
 struct Qdisc_class_common {
        u32                     classid;
+       unsigned int            filter_cnt;
        struct hlist_node       hnode;
 };
 
@@ -633,6 +634,31 @@ qdisc_class_find(const struct Qdisc_class_hash *hash, u32 id)
        return NULL;
 }
 
+static inline bool qdisc_class_in_use(const struct Qdisc_class_common *cl)
+{
+       return cl->filter_cnt > 0;
+}
+
+static inline void qdisc_class_get(struct Qdisc_class_common *cl)
+{
+       unsigned int res;
+
+       if (check_add_overflow(cl->filter_cnt, 1, &res))
+               WARN(1, "Qdisc class overflow");
+
+       cl->filter_cnt = res;
+}
+
+static inline void qdisc_class_put(struct Qdisc_class_common *cl)
+{
+       unsigned int res;
+
+       if (check_sub_overflow(cl->filter_cnt, 1, &res))
+               WARN(1, "Qdisc class underflow");
+
+       cl->filter_cnt = res;
+}
+
 static inline int tc_classid_to_hwtc(struct net_device *dev, u32 classid)
 {
        u32 hwtc = TC_H_MIN(classid) - TC_H_MIN_PRIORITY;

diff --git a/net/sched/sch_drr.c b/net/sched/sch_drr.c
index e35a4e9..1b22b3b 100644 (file)
--- a/net/sched/sch_drr.c
+++ b/net/sched/sch_drr.c
@@ -17,7 +17,6 @@
 
 struct drr_class {
        struct Qdisc_class_common       common;
-       unsigned int                    filter_cnt;
 
        struct gnet_stats_basic_sync            bstats;
        struct gnet_stats_queue         qstats;
@@ -150,7 +149,7 @@ static int drr_delete_class(struct Qdisc *sch, unsigned long arg,
        struct drr_sched *q = qdisc_priv(sch);
        struct drr_class *cl = (struct drr_class *)arg;
 
-       if (cl->filter_cnt > 0)
+       if (qdisc_class_in_use(&cl->common))
                return -EBUSY;
 
        sch_tree_lock(sch);
@@ -187,8 +186,8 @@ static unsigned long drr_bind_tcf(struct Qdisc *sch, unsigned long parent,
 {
        struct drr_class *cl = drr_find_class(sch, classid);
 
-       if (cl != NULL)
-               cl->filter_cnt++;
+       if (cl)
+               qdisc_class_get(&cl->common);
 
        return (unsigned long)cl;
 }
@@ -197,7 +196,7 @@ static void drr_unbind_tcf(struct Qdisc *sch, unsigned long arg)
 {
        struct drr_class *cl = (struct drr_class *)arg;
 
-       cl->filter_cnt--;
+       qdisc_class_put(&cl->common);
 }
 
 static int drr_graft_class(struct Qdisc *sch, unsigned long arg,

diff --git a/net/sched/sch_hfsc.c b/net/sched/sch_hfsc.c
index 70b0c58..896cb40 100644 (file)
--- a/net/sched/sch_hfsc.c
+++ b/net/sched/sch_hfsc.c
@@ -116,7 +116,6 @@ struct hfsc_class {
        struct net_rate_estimator __rcu *rate_est;
        struct tcf_proto __rcu *filter_list; /* filter list */
        struct tcf_block *block;
-       unsigned int    filter_cnt;     /* filter count */
        unsigned int    level;          /* class level in hierarchy */
 
        struct hfsc_sched *sched;       /* scheduler data */
@@ -1094,7 +1093,8 @@ hfsc_delete_class(struct Qdisc *sch, unsigned long arg,
        struct hfsc_sched *q = qdisc_priv(sch);
        struct hfsc_class *cl = (struct hfsc_class *)arg;
 
-       if (cl->level > 0 || cl->filter_cnt > 0 || cl == &q->root)
+       if (cl->level > 0 || qdisc_class_in_use(&cl->cl_common) ||
+           cl == &q->root)
                return -EBUSY;
 
        sch_tree_lock(sch);
@@ -1223,7 +1223,7 @@ hfsc_bind_tcf(struct Qdisc *sch, unsigned long parent, u32 classid)
        if (cl != NULL) {
                if (p != NULL && p->level <= cl->level)
                        return 0;
-               cl->filter_cnt++;
+               qdisc_class_get(&cl->cl_common);
        }
 
        return (unsigned long)cl;
@@ -1234,7 +1234,7 @@ hfsc_unbind_tcf(struct Qdisc *sch, unsigned long arg)
 {
        struct hfsc_class *cl = (struct hfsc_class *)arg;
 
-       cl->filter_cnt--;
+       qdisc_class_put(&cl->cl_common);
 }
 
 static struct tcf_block *hfsc_tcf_block(struct Qdisc *sch, unsigned long arg,

diff --git a/net/sched/sch_htb.c b/net/sched/sch_htb.c
index 333800a..05c8291 100644 (file)
--- a/net/sched/sch_htb.c
+++ b/net/sched/sch_htb.c
@@ -102,7 +102,6 @@ struct htb_class {
 
        struct tcf_proto __rcu  *filter_list;   /* class attached filters */
        struct tcf_block        *block;
-       int                     filter_cnt;
 
        int                     level;          /* our level (see above) */
        unsigned int            children;
@@ -1710,7 +1709,7 @@ static int htb_delete(struct Qdisc *sch, unsigned long arg,
         * tc subsys guarantee us that in htb_destroy it holds no class
         * refs so that we can remove children safely there ?
         */
-       if (cl->children || cl->filter_cnt)
+       if (cl->children || qdisc_class_in_use(&cl->common))
                return -EBUSY;
 
        if (!cl->level && htb_parent_last_child(cl))
@@ -2107,7 +2106,7 @@ static unsigned long htb_bind_filter(struct Qdisc *sch, unsigned long parent,
         * be broken by class during destroy IIUC.
         */
        if (cl)
-               cl->filter_cnt++;
+               qdisc_class_get(&cl->common);
        return (unsigned long)cl;
 }
 
@@ -2115,8 +2114,7 @@ static void htb_unbind_filter(struct Qdisc *sch, unsigned long arg)
 {
        struct htb_class *cl = (struct htb_class *)arg;
 
-       if (cl)
-               cl->filter_cnt--;
+       qdisc_class_put(&cl->common);
 }
 
 static void htb_walk(struct Qdisc *sch, struct qdisc_walker *arg)

diff --git a/net/sched/sch_qfq.c b/net/sched/sch_qfq.c
index befaf74..7addc15 100644 (file)
--- a/net/sched/sch_qfq.c
+++ b/net/sched/sch_qfq.c
@@ -130,8 +130,6 @@ struct qfq_aggregate;
 struct qfq_class {
        struct Qdisc_class_common common;
 
-       unsigned int filter_cnt;
-
        struct gnet_stats_basic_sync bstats;
        struct gnet_stats_queue qstats;
        struct net_rate_estimator __rcu *rate_est;
@@ -545,7 +543,7 @@ static int qfq_delete_class(struct Qdisc *sch, unsigned long arg,
        struct qfq_sched *q = qdisc_priv(sch);
        struct qfq_class *cl = (struct qfq_class *)arg;
 
-       if (cl->filter_cnt > 0)
+       if (qdisc_class_in_use(&cl->common))
                return -EBUSY;
 
        sch_tree_lock(sch);
@@ -580,8 +578,8 @@ static unsigned long qfq_bind_tcf(struct Qdisc *sch, unsigned long parent,
 {
        struct qfq_class *cl = qfq_find_class(sch, classid);
 
-       if (cl != NULL)
-               cl->filter_cnt++;
+       if (cl)
+               qdisc_class_get(&cl->common);
 
        return (unsigned long)cl;
 }
@@ -590,7 +588,7 @@ static void qfq_unbind_tcf(struct Qdisc *sch, unsigned long arg)
 {
        struct qfq_class *cl = (struct qfq_class *)arg;
 
-       cl->filter_cnt--;
+       qdisc_class_put(&cl->common);
 }
 
 static int qfq_graft_class(struct Qdisc *sch, unsigned long arg,