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++.
Unified · Split
| @@ -102,13 +102,15 @@ static struct list_head *mlx5_fc_counters_lookup_next(struct mlx5_core_dev *dev, · 101 unmodified lines | |||
| 102 | struct mlx5_fc_stats *fc_stats = &dev->priv.fc_stats; | 102 | struct mlx5_fc_stats *fc_stats = &dev->priv.fc_stats; |
| 103 | unsigned long next_id = (unsigned long)id + 1; | 103 | unsigned long next_id = (unsigned long)id + 1; |
| 104 | struct mlx5_fc *counter; | 104 | struct mlx5_fc *counter; |
| 105 | unsigned long tmp; | ||
| 105 | 106 | ||
| 106 | rcu_read_lock(); | 107 | rcu_read_lock(); |
| 107 | /* skip counters that are in idr, but not yet in counters list */ | 108 | /* skip counters that are in idr, but not yet in counters list */ |
| 108 | while ((counter = idr_get_next_ul(&fc_stats->counters_idr, | 109 | idr_for_each_entry_continue_ul(&fc_stats->counters_idr, |
| 109 | &next_id)) != NULL && | 110 | counter, tmp, next_id) { |
| 110 | list_empty(&counter->list)) | 111 | if (!list_empty(&counter->list)) |
| 111 | next_id++; | 112 | break; |
| 113 | } | ||
| 112 | rcu_read_unlock(); | 114 | rcu_read_unlock(); |
| 113 | 115 | ||
| 114 | return counter ? &counter->list : &fc_stats->counters; | 116 | return counter ? &counter->list : &fc_stats->counters; |
| @@ -216,6 +216,20 @@ static inline void idr_preload_end(void) · 215 unmodified lines | |||
| 216 | entry; \ | 216 | entry; \ |
| 217 | ++id, (entry) = idr_get_next((idr), &(id))) | 217 | ++id, (entry) = idr_get_next((idr), &(id))) |
| 218 | 218 | ||
| 219 | /** | ||
| 220 | * idr_for_each_entry_continue_ul() - Continue iteration over an IDR's elements of a given type | ||
| 221 | * @idr: IDR handle. | ||
| 222 | * @entry: The type * to use as a cursor. | ||
| 223 | * @tmp: A temporary placeholder for ID. | ||
| 224 | * @id: Entry ID. | ||
| 225 | * | ||
| 226 | * Continue to iterate over entries, continuing after the current position. | ||
| 227 | */ | ||
| 228 | #define idr_for_each_entry_continue_ul(idr, entry, tmp, id) \ | ||
| 229 | for (tmp = id; \ | ||
| 230 | tmp <= id && ((entry) = idr_get_next_ul(idr, &(id))) != NULL; \ | ||
| 231 | tmp = id, ++id) | ||
| 232 | |||
| 219 | /* | 233 | /* |
| 220 | * IDA - ID Allocator, use when translation from id to pointer isn't necessary. | 234 | * IDA - ID Allocator, use when translation from id to pointer isn't necessary. |
| 221 | */ | 235 | */ |
| @@ -524,24 +524,6 @@ static struct cls_fl_filter *__fl_get(struct cls_fl_head *head, u32 handle) · 523 unmodified lines | |||
| 524 | return f; | 524 | return f; |
| 525 | } | 525 | } |
| 526 | 526 | ||
| 527 | static struct cls_fl_filter *fl_get_next_filter(struct tcf_proto *tp, | ||
| 528 | unsigned long *handle) | ||
| 529 | { | ||
| 530 | struct cls_fl_head *head = fl_head_dereference(tp); | ||
| 531 | struct cls_fl_filter *f; | ||
| 532 | |||
| 533 | rcu_read_lock(); | ||
| 534 | while ((f = idr_get_next_ul(&head->handle_idr, handle))) { | ||
| 535 | /* don't return filters that are being deleted */ | ||
| 536 | if (refcount_inc_not_zero(&f->refcnt)) | ||
| 537 | break; | ||
| 538 | ++(*handle); | ||
| 539 | } | ||
| 540 | rcu_read_unlock(); | ||
| 541 | |||
| 542 | return f; | ||
| 543 | } | ||
| 544 | |||
| 545 | static int __fl_delete(struct tcf_proto *tp, struct cls_fl_filter *f, | 527 | static int __fl_delete(struct tcf_proto *tp, struct cls_fl_filter *f, |
| 546 | bool *last, bool rtnl_held, | 528 | bool *last, bool rtnl_held, |
| 547 | struct netlink_ext_ack *extack) | 529 | struct netlink_ext_ack *extack) |
| @@ -1691,20 +1673,25 @@ static int fl_delete(struct tcf_proto *tp, void *arg, bool *last, · 1143 unmodified lines | |||
| 1691 | static void fl_walk(struct tcf_proto *tp, struct tcf_walker *arg, | 1673 | static void fl_walk(struct tcf_proto *tp, struct tcf_walker *arg, |
| 1692 | bool rtnl_held) | 1674 | bool rtnl_held) |
| 1693 | { | 1675 | { |
| 1676 | struct cls_fl_head *head = fl_head_dereference(tp); | ||
| 1677 | unsigned long id = arg->cookie, tmp; | ||
| 1694 | struct cls_fl_filter *f; | 1678 | struct cls_fl_filter *f; |
| 1695 | 1679 | ||
| 1696 | arg->count = arg->skip; | 1680 | arg->count = arg->skip; |
| 1697 | 1681 | ||
| 1698 | while ((f = fl_get_next_filter(tp, &arg->cookie)) != NULL) { | 1682 | idr_for_each_entry_continue_ul(&head->handle_idr, f, tmp, id) { |
| 1683 | /* don't return filters that are being deleted */ | ||
| 1684 | if (!refcount_inc_not_zero(&f->refcnt)) | ||
| 1685 | continue; | ||
| 1699 | if (arg->fn(tp, f, arg) < 0) { | 1686 | if (arg->fn(tp, f, arg) < 0) { |
| 1700 | __fl_put(f); | 1687 | __fl_put(f); |
| 1701 | arg->stop = 1; | 1688 | arg->stop = 1; |
| 1702 | break; | 1689 | break; |
| 1703 | } | 1690 | } |
| 1704 | __fl_put(f); | 1691 | __fl_put(f); |
| 1705 | arg->cookie++; | ||
| 1706 | arg->count++; | 1692 | arg->count++; |
| 1707 | } | 1693 | } |
| 1694 | arg->cookie = id; | ||
| 1708 | } | 1695 | } |
| 1709 | 1696 | ||
| 1710 | static struct cls_fl_filter * | 1697 | static struct cls_fl_filter * |