Re: 2.6.21-git11: BUG in loop.ko

["Ken Chen" <kenchen@google.com>]

On 5/9/07, Andrew Morton <akpm@linux-foundation.org> wrote:
> On Wed, 09 May 2007 16:52:41 -0700
> Jeremy Fitzhardinge <jeremy@goop.org> 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@google.com>


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)