From: Tejun Heo <tj@kernel.org>
To: akpm@linux-foundation.org
Cc: linux-kernel@vger.kernel.org, Tejun Heo <tj@kernel.org>,
KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>,
stable@vger.kernel.org
Subject: [PATCH 01/77] idr: fix a subtle bug in idr_get_next()
Date: Wed, 6 Feb 2013 11:39:33 -0800 [thread overview]
Message-ID: <1360179649-22465-2-git-send-email-tj@kernel.org> (raw)
In-Reply-To: <1360179649-22465-1-git-send-email-tj@kernel.org>
The iteration logic of idr_get_next() is borrowed mostly verbatim from
idr_for_each(). It walks down the tree looking for the slot matching
the current ID. If the matching slot is not found, the ID is
incremented by the distance of single slot at the given level and
repeats.
The implementation assumes that during the whole iteration id is
aligned to the layer boundaries of the level closest to the leaf,
which is true for all iterations starting from zero or an existing
element and thus is fine for idr_for_each().
However, idr_get_next() may be given any point and if the starting id
hits in the middle of a non-existent layer, increment to the next
layer will end up skipping the same offset into it. For example, an
IDR with IDs filled between [64, 127] would look like the following.
[ 0 64 ... ]
/----/ |
| |
NULL [ 64 ... 127 ]
If idr_get_next() is called with 63 as the starting point, it will try
to follow down the pointer from 0. As it is NULL, it will then try to
proceed to the next slot in the same level by adding the slot distance
at that level which is 64 - making the next try 127. It goes around
the loop and finds and returns 127 skipping [64, 126].
Note that this bug also triggers in idr_for_each_entry() loop which
deletes during iteration as deletions can make layers go away leaving
the iteration with unaligned ID into missing layers.
Fix it by ensuring proceeding to the next slot doesn't carry over the
unaligned offset - ie. use round_up(id + 1, slot_distance) instead of
id += slot_distance.
Signed-off-by: Tejun Heo <tj@kernel.org>
Reported-by: David Teigland <teigland@redhat.com>
Cc: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
Cc: stable@vger.kernel.org
---
lib/idr.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/lib/idr.c b/lib/idr.c
index 6482390..ca5aa00 100644
--- a/lib/idr.c
+++ b/lib/idr.c
@@ -625,7 +625,14 @@ void *idr_get_next(struct idr *idp, int *nextidp)
return p;
}
- id += 1 << n;
+ /*
+ * Proceed to the next layer at the current level. Unlike
+ * idr_for_each(), @id isn't guaranteed to be aligned to
+ * layer boundary at this point and adding 1 << n may
+ * incorrectly skip IDs. Make sure we jump to the
+ * beginning of the next layer using round_up().
+ */
+ id = round_up(id + 1, 1 << n);
while (n < fls(id)) {
n += IDR_BITS;
p = *--paa;
--
1.8.1
next prev parent reply other threads:[~2013-02-06 19:49 UTC|newest]
Thread overview: 106+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-02-06 19:39 [PATCHSET] idr: deprecate idr_remova_all() and add idr_alloc() Tejun Heo
2013-02-06 19:39 ` Tejun Heo [this message]
2013-02-06 19:39 ` [PATCH 02/77] idr: make idr_destroy() imply idr_remove_all() Tejun Heo
2013-02-06 19:39 ` [PATCH 03/77] atm/nicstar: don't use idr_remove_all() Tejun Heo
2013-02-06 19:39 ` [PATCH 04/77] block/loop: " Tejun Heo
2013-02-06 19:39 ` [PATCH 05/77] firewire: " Tejun Heo
2013-02-06 19:39 ` [PATCH 06/77] drm: " Tejun Heo
2013-02-06 19:39 ` [PATCH 07/77] dm: " Tejun Heo
2013-02-06 19:39 ` [PATCH 08/77] remoteproc: " Tejun Heo
2013-02-06 19:39 ` [PATCH 09/77] rpmsg: " Tejun Heo
2013-02-06 19:39 ` [PATCH 10/77] dlm: use idr_for_each_entry() in recover_idr_clear() error path Tejun Heo
2013-02-06 19:39 ` [PATCH 11/77] dlm: don't use idr_remove_all() Tejun Heo
2013-02-06 19:39 ` [PATCH 12/77] nfs: idr_destroy() no longer needs idr_remove_all() Tejun Heo
2013-02-06 19:39 ` [PATCH 13/77] inotify: don't use idr_remove_all() Tejun Heo
2013-02-06 19:39 ` [PATCH 14/77] cgroup: " Tejun Heo
2013-02-07 1:29 ` Li Zefan
2013-02-06 19:39 ` [PATCH 15/77] idr: deprecate idr_remove_all() Tejun Heo
2013-02-06 19:39 ` [PATCH 16/77] idr: cosmetic updates to struct / initializer definitions Tejun Heo
2013-02-06 19:39 ` [PATCH 17/77] idr: relocate idr_for_each_entry() and reorganize id[r|a]_get_new() Tejun Heo
2013-02-06 19:39 ` [PATCH 18/77] idr: remove _idr_rc_to_errno() hack Tejun Heo
2013-02-06 19:39 ` [PATCH 19/77] idr: refactor idr_get_new_above() Tejun Heo
2013-02-06 19:39 ` [PATCH 20/77] idr: implement idr_preload[_end]() and idr_alloc() Tejun Heo
2013-02-07 19:53 ` [PATCH v3 " Tejun Heo
2013-02-06 19:39 ` [PATCH 21/77] block: fix synchronization and limit check in blk_alloc_devt() Tejun Heo
2013-02-06 22:24 ` Andrew Morton
2013-02-06 22:27 ` Tejun Heo
2013-02-06 22:32 ` Andrew Morton
2013-02-06 22:33 ` Tejun Heo
2013-02-06 19:39 ` [PATCH 22/77] block: convert to idr_alloc() Tejun Heo
2013-02-06 19:39 ` [PATCH 23/77] block/loop: " Tejun Heo
2013-02-07 18:25 ` [PATCH 22.5/77] block/loop: fix error return value in loop_add() Tejun Heo
2013-02-07 18:26 ` [PATCH v2 23/77] block/loop: convert to idr_alloc() Tejun Heo
2013-02-06 19:39 ` [PATCH 24/77] atm/nicstar: " Tejun Heo
2013-02-06 19:39 ` [PATCH 25/77] drbd: " Tejun Heo
2013-02-06 19:39 ` [PATCH 26/77] dca: " Tejun Heo
2013-02-06 19:39 ` [PATCH 27/77] dmaengine: " Tejun Heo
2013-02-06 19:40 ` [PATCH 28/77] firewire: add minor number range check to fw_device_init() Tejun Heo
2013-02-06 19:40 ` [PATCH 29/77] firewire: convert to idr_alloc() Tejun Heo
2013-02-06 19:40 ` [PATCH 30/77] gpio: " Tejun Heo
2013-02-06 19:40 ` [PATCH 31/77] drm: " Tejun Heo
2013-02-06 19:40 ` [PATCH 32/77] drm/exynos: " Tejun Heo
2013-02-06 19:40 ` [PATCH 33/77] drm/i915: " Tejun Heo
2013-02-06 19:40 ` [PATCH 34/77] drm/sis: " Tejun Heo
2013-02-06 19:40 ` [PATCH 35/77] drm/via: " Tejun Heo
2013-02-06 19:40 ` [PATCH 36/77] drm/vmwgfx: " Tejun Heo
2013-02-06 19:40 ` [PATCH 37/77] i2c: " Tejun Heo
2013-02-07 15:28 ` Mark Brown
2013-02-07 16:32 ` Tejun Heo
2013-02-07 16:39 ` Mark Brown
2013-02-07 16:55 ` [PATCH v2] " Tejun Heo
2013-02-07 18:52 ` Mark Brown
2013-02-08 12:10 ` Mark Brown
2013-02-10 11:47 ` Wolfram Sang
2013-02-12 17:34 ` [PATCH -mm] i2c: style cleanups after idr_alloc() conversion Tejun Heo
2013-02-12 17:36 ` Tejun Heo
2013-02-12 18:00 ` Jean Delvare
2013-02-13 20:42 ` Wolfram Sang
2013-02-06 19:40 ` [PATCH 38/77] IB/core: convert to idr_alloc() Tejun Heo
2013-02-06 19:40 ` [PATCH 39/77] IB/amso1100: " Tejun Heo
2013-02-06 19:40 ` [PATCH 40/77] IB/cxgb3: " Tejun Heo
2013-02-06 19:40 ` [PATCH 41/77] IB/cxgb4: " Tejun Heo
2013-02-06 19:40 ` [PATCH 42/77] IB/ehca: " Tejun Heo
2013-02-06 19:40 ` [PATCH 43/77] IB/ipath: " Tejun Heo
2013-02-06 19:40 ` [PATCH 44/77] IB/mlx4: " Tejun Heo
2013-02-06 19:40 ` [PATCH 45/77] IB/ocrdma: " Tejun Heo
2013-02-06 19:40 ` [PATCH 46/77] IB/qib: " Tejun Heo
2013-02-06 19:40 ` [PATCH 47/77] dm: " Tejun Heo
2013-02-06 19:40 ` [PATCH 48/77] memstick: " Tejun Heo
2013-02-06 19:40 ` [PATCH 49/77] mfd: " Tejun Heo
2013-02-06 19:40 ` [PATCH 50/77] misc/c2port: " Tejun Heo
2013-02-06 19:40 ` [PATCH 51/77] misc/tifm_core: " Tejun Heo
2013-02-06 19:40 ` [PATCH 52/77] mmc: " Tejun Heo
2013-02-06 19:40 ` [PATCH 53/77] mtd: " Tejun Heo
2013-02-06 19:40 ` [PATCH 54/77] macvtap: " Tejun Heo
2013-02-06 19:40 ` [PATCH 55/77] ppp: " Tejun Heo
2013-02-06 19:40 ` [PATCH 56/77] power: " Tejun Heo
2013-02-06 19:40 ` [PATCH 57/77] pps: " Tejun Heo
2013-02-06 19:40 ` [PATCH 58/77] remoteproc: " Tejun Heo
2013-02-06 19:40 ` [PATCH 59/77] rpmsg: " Tejun Heo
2013-02-06 19:40 ` [PATCH 60/77] scsi/bfa: " Tejun Heo
2013-02-06 19:40 ` [PATCH 61/77] scsi: " Tejun Heo
2013-02-06 19:40 ` [PATCH 62/77] target/iscsi: " Tejun Heo
2013-02-06 19:40 ` [PATCH 63/77] scsi/lpfc: " Tejun Heo
2013-02-11 22:47 ` James Smart
2013-02-06 19:40 ` [PATCH 64/77] thermal: " Tejun Heo
2013-02-06 19:40 ` [PATCH 65/77] uio: " Tejun Heo
2013-02-06 19:40 ` [PATCH 66/77] vfio: " Tejun Heo
2013-02-06 19:40 ` [PATCH 67/77] dlm: " Tejun Heo
2013-03-11 19:29 ` David Teigland
2013-03-11 20:28 ` Tejun Heo
2013-03-12 15:17 ` David Teigland
2013-03-12 21:22 ` [PATCH] idr: idr_alloc() shouldn't trigger lowmem warning when preloaded Tejun Heo
2013-02-06 19:40 ` [PATCH 68/77] inotify: convert to idr_alloc() Tejun Heo
2013-02-06 19:40 ` [PATCH 69/77] ocfs2: " Tejun Heo
2013-02-06 19:40 ` [PATCH 70/77] ipc: " Tejun Heo
2013-02-07 19:43 ` [PATCH v2 " Tejun Heo
2013-02-06 19:40 ` [PATCH 71/77] cgroup: " Tejun Heo
2013-02-06 19:40 ` [PATCH 72/77] events: " Tejun Heo
2013-02-06 19:40 ` [PATCH 73/77] posix-timers: " Tejun Heo
2013-02-06 19:40 ` [PATCH 74/77] net/9p: " Tejun Heo
2013-02-06 19:40 ` [PATCH 75/77] mac80211: " Tejun Heo
2013-02-06 19:40 ` [PATCH 76/77] sctp: " Tejun Heo
2013-02-06 20:07 ` Vlad Yasevich
2013-02-07 14:49 ` Neil Horman
2013-02-06 19:40 ` [PATCH 77/77] nfs4client: " Tejun Heo
2013-02-08 4:04 ` [PATCHSET] idr: deprecate idr_remova_all() and add idr_alloc() Dave Airlie
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1360179649-22465-2-git-send-email-tj@kernel.org \
--to=tj@kernel.org \
--cc=akpm@linux-foundation.org \
--cc=kamezawa.hiroyu@jp.fujitsu.com \
--cc=linux-kernel@vger.kernel.org \
--cc=stable@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).