public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Tejun Heo <tj@kernel.org>
To: Andrew Morton <akpm@linux-foundation.org>,
	Greg KH <gregkh@suse.de>, Eric Paris <eparis@redhat.com>,
	lkml <linux-kernel@vger.kernel.org>,
	Xiaotian Feng <xtfeng@gmail.com>,
	Dave Hansen <dave@linux.vnet.ibm.com>,
	Serge Hallyn <serue@us.ibm.com>
Subject: [PATCH] idr: fix a critical misallocation bug, take#2
Date: Fri, 12 Feb 2010 16:16:07 +0900	[thread overview]
Message-ID: <4B750037.5020608@kernel.org> (raw)

This is retry of reverted 859ddf09743a8cc680af33f7259ccd0fd36bfe9d
which contained two bugs.

* pa[idp->layers] should be cleared even if it's not used by
  sub_alloc() because it's used by mark idr_mark_full().

* The original condition check also assigned pa[l] to p which the new
  code didn't do thus leaving p pointing at the wrong layer.

Both problems have been fixed and the idr code has received good
amount testing using userland testing setup where simple bitmap
allocator is run parallel to verify the result of idr allocation.

The bug this patch fixes is caused by sub_alloc() optimization path
bypassing out-of-room condition check and restarting allocation loop
with starting value higher than maximum allowed value.  For detailed
description, please read commit message of 859ddf09.

Signed-off-by: Tejun Heo <tj@kernel.org>
Based-on-patch-from: Eric Paris <eparis@redhat.com>
Reported-by: Eric Paris <eparis@redhat.com>
Tested-by: Stefan Lippers-Hollmann <s.l-h@gmx.de>
---
Heh... Embarrassingly, it turns out Eric's original patch is correct
and all I did was adding two mistakes. :-) I'm pretty sure this is the
correct fix and have tested it quite extensively.  But, given the
fragility of this thing, Andrew, can you please put it in your tree
and push it after 2.6.33 merge window opens?  Greg, please don't put
this into -stable until at least 2.6.33-rc2 seems okay.

Thank you.

 lib/idr.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/lib/idr.c b/lib/idr.c
index 1cac726..0dc7822 100644
--- a/lib/idr.c
+++ b/lib/idr.c
@@ -156,10 +156,12 @@ static int sub_alloc(struct idr *idp, int *starting_id, struct idr_layer **pa)
 			id = (id | ((1 << (IDR_BITS * l)) - 1)) + 1;
 
 			/* if already at the top layer, we need to grow */
-			if (!(p = pa[l])) {
+			if (id >= 1 << (idp->layers * IDR_BITS)) {
 				*starting_id = id;
 				return IDR_NEED_TO_GROW;
 			}
+			p = pa[l];
+			BUG_ON(!p);
 
 			/* If we need to go up one layer, continue the
 			 * loop; otherwise, restart from the top.

             reply	other threads:[~2010-02-12  7:09 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-02-12  7:16 Tejun Heo [this message]
2010-02-12 14:36 ` [PATCH] idr: fix a critical misallocation bug, take#2 Serge E. Hallyn

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=4B750037.5020608@kernel.org \
    --to=tj@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=dave@linux.vnet.ibm.com \
    --cc=eparis@redhat.com \
    --cc=gregkh@suse.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=serue@us.ibm.com \
    --cc=xtfeng@gmail.com \
    /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