Re: 2.6.21-git11: BUG in loop.ko

[Ken Chen]

On 5/9/07, Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> wrote:
> On Wed, 09 May 2007 16:52:41 -0700
> Jeremy Fitzhardinge <jeremy@xxxxxxxx> wrote:
>
> > Seems to be getting a 0 refcount.  I don't see anything in the recent
> > changes which might cause this, but this is relatively new behaviour.
> > It was working for me in the 2.6.21-pre time period, but I haven't tried
> > this since 2.6.21 was released.
> >
> > The BUG is actually triggered by the __module_get(THIS_MODULE) in
> > loop_set_fd.
>
> A few people have been playing with module refcounting lately.  Did you
> work out a reproduce-it recipe?

Ah, it's a mis-understanding on what kobj_probe_t function is suppose
to return on success.  When we open loop device that has not been
initialized, we probe it via:

do_open
 get_gendisk
   kobj_lookup
     loop_probe

Notice that in kobj_lookup(), when p->probe() returns non-zero value
(I presume it is an -ERRNO), it breaks out of the loop and propagate
the return value, otherwise, loops back to the beginning of the for
loop and retry, and in there get_disk() will be called via p->lock()
to get a ref against the module.

kobj_look_up(...) {
retry:
       mutex_lock(domain->lock);
       for (p = domain->probes[MAJOR(dev) % 255]; p; p = p->next) {
       ...
       if (kobj)
               return kobj;
       goto retry;
}

So loop_probe() mistakenly returned wrong status and leads to future
oops on inconsistent module ref count.  The following patch fixes the
issue.

Signed-off-by: Ken Chen <kenchen@xxxxxxxxxx>


diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index 18cdd8c..40f7bc2 100644
--- a/drivers/block/loop.c
+++ b/drivers/block/loop.c
@@ -1460,6 +1460,7 @@ static void loop_del_one(struct loop_device *lo)
	kfree(lo);
}

+/* return NULL for success, or return non-zero value if there are error */
static struct kobject *loop_probe(dev_t dev, int *part, void *data)
{
	unsigned int number = dev & MINORMASK;
@@ -1474,8 +1475,8 @@ static struct kobject *loop_probe
	*part = 0;
	if (IS_ERR(lo))
		return (void *)lo;
-	else
-		return &lo->lo_disk->kobj;
+
+	return NULL;
}

static int __init loop_init(void)
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/