Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

idr: remove MAX_IDR_MASK and move left MAX_IDR_* into idr.c

MAX_IDR_MASK is another weirdness in the idr interface. As idr covers
whole positive integer range, it's defined as 0x7fffffff or INT_MAX.

Its usage in idr_find(), idr_replace() and idr_remove() is bizarre.
They basically mask off the sign bit and operate on the rest, so if
the caller, by accident, passes in a negative number, the sign bit
will be masked off and the remaining part will be used as if that was
the input, which is worse than crashing.

The constant is visible in idr.h and there are several users in the
kernel.

* drivers/i2c/i2c-core.c:i2c_add_numbered_adapter()

Basically used to test if adap->nr is a negative number which isn't
-1 and returns -EINVAL if so. idr_alloc() already has negative
@start checking (w/ WARN_ON_ONCE), so this can go away.

* drivers/infiniband/core/cm.c:cm_alloc_id()
drivers/infiniband/hw/mlx4/cm.c:id_map_alloc()

Used to wrap cyclic @start. Can be replaced with max(next, 0).
Note that this type of cyclic allocation using idr is buggy. These
are prone to spurious -ENOSPC failure after the first wraparound.

* fs/super.c:get_anon_bdev()

The ID allocated from ida is masked off before being tested whether
it's inside valid range. ida allocated ID can never be a negative
number and the masking is unnecessary.

Update idr_*() functions to fail with -EINVAL when negative @id is
specified and update other MAX_IDR_MASK users as described above.

This leaves MAX_IDR_MASK without any user, remove it and relocate
other MAX_IDR_* constants to lib/idr.c.

Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Jean Delvare <khali@linux-fr.org>
Cc: Roland Dreier <roland@kernel.org>
Cc: Sean Hefty <sean.hefty@intel.com>
Cc: Hal Rosenstock <hal.rosenstock@gmail.com>
Cc: "Marciniszyn, Mike" <mike.marciniszyn@intel.com>
Cc: Jack Morgenstein <jackm@dev.mellanox.co.il>
Cc: Or Gerlitz <ogerlitz@mellanox.com>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Acked-by: Wolfram Sang <wolfram@the-dreams.de>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

authored by

Tejun Heo and committed by
Linus Torvalds
e8c8d1bc 326cf0f0

+20 -22
-2
drivers/i2c/i2c-core.c
··· 979 979 980 980 if (adap->nr == -1) /* -1 means dynamically assign bus id */ 981 981 return i2c_add_adapter(adap); 982 - if (adap->nr & ~MAX_IDR_MASK) 983 - return -EINVAL; 984 982 985 983 mutex_lock(&core_lock); 986 984 id = idr_alloc(&i2c_adapter_idr, adap, adap->nr, adap->nr + 1,
+1 -1
drivers/infiniband/core/cm.c
··· 390 390 391 391 id = idr_alloc(&cm.local_id_table, cm_id_priv, next_id, 0, GFP_NOWAIT); 392 392 if (id >= 0) 393 - next_id = ((unsigned) id + 1) & MAX_IDR_MASK; 393 + next_id = max(id + 1, 0); 394 394 395 395 spin_unlock_irqrestore(&cm.lock, flags); 396 396 idr_preload_end();
+1 -1
drivers/infiniband/hw/mlx4/cm.c
··· 225 225 226 226 ret = idr_alloc(&sriov->pv_id_table, ent, next_id, 0, GFP_NOWAIT); 227 227 if (ret >= 0) { 228 - next_id = ((unsigned)ret + 1) & MAX_IDR_MASK; 228 + next_id = max(ret + 1, 0); 229 229 ent->pv_cm_id = (u32)ret; 230 230 sl_id_map_add(ibdev, ent); 231 231 list_add_tail(&ent->list, &sriov->cm_list);
+1 -1
fs/super.c
··· 842 842 else if (error) 843 843 return -EAGAIN; 844 844 845 - if ((dev & MAX_IDR_MASK) == (1 << MINORBITS)) { 845 + if (dev == (1 << MINORBITS)) { 846 846 spin_lock(&unnamed_dev_lock); 847 847 ida_remove(&unnamed_dev_ida, dev); 848 848 if (unnamed_dev_start > dev)
-10
include/linux/idr.h
··· 38 38 #define IDR_SIZE (1 << IDR_BITS) 39 39 #define IDR_MASK ((1 << IDR_BITS)-1) 40 40 41 - #define MAX_IDR_SHIFT (sizeof(int)*8 - 1) 42 - #define MAX_IDR_BIT (1U << MAX_IDR_SHIFT) 43 - #define MAX_IDR_MASK (MAX_IDR_BIT - 1) 44 - 45 - /* Leave the possibility of an incomplete final layer */ 46 - #define MAX_IDR_LEVEL ((MAX_IDR_SHIFT + IDR_BITS - 1) / IDR_BITS) 47 - 48 - /* Number of id_layer structs to leave in free list */ 49 - #define MAX_IDR_FREE (MAX_IDR_LEVEL * 2) 50 - 51 41 struct idr_layer { 52 42 unsigned long bitmap; /* A zero bit means "space here" */ 53 43 struct idr_layer __rcu *ary[1<<IDR_BITS];
+17 -7
lib/idr.c
··· 38 38 #include <linux/percpu.h> 39 39 #include <linux/hardirq.h> 40 40 41 + #define MAX_IDR_SHIFT (sizeof(int) * 8 - 1) 42 + #define MAX_IDR_BIT (1U << MAX_IDR_SHIFT) 43 + 44 + /* Leave the possibility of an incomplete final layer */ 45 + #define MAX_IDR_LEVEL ((MAX_IDR_SHIFT + IDR_BITS - 1) / IDR_BITS) 46 + 47 + /* Number of id_layer structs to leave in free list */ 48 + #define MAX_IDR_FREE (MAX_IDR_LEVEL * 2) 49 + 41 50 static struct kmem_cache *idr_layer_cache; 42 51 static DEFINE_PER_CPU(struct idr_layer *, idr_preload_head); 43 52 static DEFINE_PER_CPU(int, idr_preload_cnt); ··· 551 542 struct idr_layer *p; 552 543 struct idr_layer *to_free; 553 544 554 - /* Mask off upper bits we don't use for the search. */ 555 - id &= MAX_IDR_MASK; 545 + if (WARN_ON_ONCE(id < 0)) 546 + return; 556 547 557 548 sub_remove(idp, (idp->layers - 1) * IDR_BITS, id); 558 549 if (idp->top && idp->top->count == 1 && (idp->layers > 1) && ··· 659 650 int n; 660 651 struct idr_layer *p; 661 652 653 + if (WARN_ON_ONCE(id < 0)) 654 + return NULL; 655 + 662 656 p = rcu_dereference_raw(idp->top); 663 657 if (!p) 664 658 return NULL; 665 659 n = (p->layer+1) * IDR_BITS; 666 - 667 - /* Mask off upper bits we don't use for the search. */ 668 - id &= MAX_IDR_MASK; 669 660 670 661 if (id > idr_max(p->layer + 1)) 671 662 return NULL; ··· 808 799 int n; 809 800 struct idr_layer *p, *old_p; 810 801 802 + if (WARN_ON_ONCE(id < 0)) 803 + return ERR_PTR(-EINVAL); 804 + 811 805 p = idp->top; 812 806 if (!p) 813 807 return ERR_PTR(-EINVAL); 814 808 815 809 n = (p->layer+1) * IDR_BITS; 816 - 817 - id &= MAX_IDR_MASK; 818 810 819 811 if (id >= (1 << n)) 820 812 return ERR_PTR(-EINVAL);