idr: introduce idr_for_each_entry_continue_ul()

Similarly, other callers of idr_get_next_ul() suffer the same
overflow bug as they don't handle it properly either.

Introduce idr_for_each_entry_continue_ul() to help these callers
iterate from a given ID.

cls_flower needs more care here because it still has overflow when
does arg->cookie++, we have to fold its nested loops into one
and remove the arg->cookie++.

Fixes: 01683a146999 ("net: sched: refactor flower walk to iterate over idr")
Fixes: 12d6066c3b29 ("net/mlx5: Add flow counters idr")
Reported-by: Li Shuang <shuali@redhat.com>
Cc: Davide Caratti <dcaratti@redhat.com>
Cc: Vlad Buslov <vladbu@mellanox.com>
Cc: Chris Mi <chrism@mellanox.com>
Cc: Matthew Wilcox <willy@infradead.org>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
Tested-by: Davide Caratti <dcaratti@redhat.com>
Signed-off-by: David S. Miller <davem@davemloft.net>

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/fs_counters.c b/drivers/net/ethernet/mellanox/mlx5/core/fs_counters.c
index c6c28f56aa2942cee106ce298d1c3f59d8462914..b3762123a69c2188f1745965aaeeb80ca8d80bf6 100644 (file)
--- a/drivers/net/ethernet/mellanox/mlx5/core/fs_counters.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/fs_counters.c
@@ -102,13 +102,15 @@ static struct list_head *mlx5_fc_counters_lookup_next(struct mlx5_core_dev *dev,
        struct mlx5_fc_stats *fc_stats = &dev->priv.fc_stats;
        unsigned long next_id = (unsigned long)id + 1;
        struct mlx5_fc *counter;
+       unsigned long tmp;
 
        rcu_read_lock();
        /* skip counters that are in idr, but not yet in counters list */
-       while ((counter = idr_get_next_ul(&fc_stats->counters_idr,
-                                         &next_id)) != NULL &&
-              list_empty(&counter->list))
-               next_id++;
+       idr_for_each_entry_continue_ul(&fc_stats->counters_idr,
+                                      counter, tmp, next_id) {
+               if (!list_empty(&counter->list))
+                       break;
+       }
        rcu_read_unlock();
 
        return counter ? &counter->list : &fc_stats->counters;

diff --git a/include/linux/idr.h b/include/linux/idr.h
index 68528a72d10d4dfa41ed7aff407875436795130e..4ec8986e5dfb61c7f2fd47abd2563893bb4deaa0 100644 (file)
--- a/include/linux/idr.h
+++ b/include/linux/idr.h
@@ -216,6 +216,20 @@ static inline void idr_preload_end(void)
             entry;                                                     \
             ++id, (entry) = idr_get_next((idr), &(id)))
 
+/**
+ * idr_for_each_entry_continue_ul() - Continue iteration over an IDR's elements of a given type
+ * @idr: IDR handle.
+ * @entry: The type * to use as a cursor.
+ * @tmp: A temporary placeholder for ID.
+ * @id: Entry ID.
+ *
+ * Continue to iterate over entries, continuing after the current position.
+ */
+#define idr_for_each_entry_continue_ul(idr, entry, tmp, id)            \
+       for (tmp = id;                                                  \
+            tmp <= id && ((entry) = idr_get_next_ul(idr, &(id))) != NULL; \
+            tmp = id, ++id)
+
 /*
  * IDA - ID Allocator, use when translation from id to pointer isn't necessary.
  */

diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
index eedd5786c0844a363c081391b5afc679c89b0843..fdeede3af72e0595ae054df88aad4d6cd2e512b7 100644 (file)
--- a/net/sched/cls_flower.c
+++ b/net/sched/cls_flower.c
@@ -524,24 +524,6 @@ static struct cls_fl_filter *__fl_get(struct cls_fl_head *head, u32 handle)
        return f;
 }
 
-static struct cls_fl_filter *fl_get_next_filter(struct tcf_proto *tp,
-                                               unsigned long *handle)
-{
-       struct cls_fl_head *head = fl_head_dereference(tp);
-       struct cls_fl_filter *f;
-
-       rcu_read_lock();
-       while ((f = idr_get_next_ul(&head->handle_idr, handle))) {
-               /* don't return filters that are being deleted */
-               if (refcount_inc_not_zero(&f->refcnt))
-                       break;
-               ++(*handle);
-       }
-       rcu_read_unlock();
-
-       return f;
-}
-
 static int __fl_delete(struct tcf_proto *tp, struct cls_fl_filter *f,
                       bool *last, bool rtnl_held,
                       struct netlink_ext_ack *extack)
@@ -1691,20 +1673,25 @@ static int fl_delete(struct tcf_proto *tp, void *arg, bool *last,
 static void fl_walk(struct tcf_proto *tp, struct tcf_walker *arg,
                    bool rtnl_held)
 {
+       struct cls_fl_head *head = fl_head_dereference(tp);
+       unsigned long id = arg->cookie, tmp;
        struct cls_fl_filter *f;
 
        arg->count = arg->skip;
 
-       while ((f = fl_get_next_filter(tp, &arg->cookie)) != NULL) {
+       idr_for_each_entry_continue_ul(&head->handle_idr, f, tmp, id) {
+               /* don't return filters that are being deleted */
+               if (!refcount_inc_not_zero(&f->refcnt))
+                       continue;
                if (arg->fn(tp, f, arg) < 0) {
                        __fl_put(f);
                        arg->stop = 1;
                        break;
                }
                __fl_put(f);
-               arg->cookie++;
                arg->count++;
        }
+       arg->cookie = id;
 }
 
 static struct cls_fl_filter *