Messages in this thread | | | Date | Fri, 2 Jul 2004 15:47:20 -0700 | From | Greg KH <> | Subject | Re: [PATCH] CDRW packet writing support for 2.6.7-bk13 |
| |
On Fri, Jul 02, 2004 at 03:08:19PM -0700, Andrew Morton wrote: > Peter Osterlund <petero2@telia.com> wrote: > > > > This code leaks a module reference. The patch below fixes it. I'm not > > sure it's correct, but do_open() also does module_put() after > > put_disk(). > > > > Signed-off-by: Peter Osterlund <petero2@telia.com> > > > > --- > > > > linux-petero/fs/partitions/check.c | 1 + > > 1 files changed, 1 insertion(+) > > > > diff -puN fs/partitions/check.c~packet-refcnt fs/partitions/check.c > > --- linux/fs/partitions/check.c~packet-refcnt 2004-07-02 23:18:22.000000000 +0200 > > +++ linux-petero/fs/partitions/check.c 2004-07-02 23:18:23.000000000 +0200 > > @@ -151,6 +151,7 @@ const char *__bdevname(dev_t dev, char * > > if (disk) { > > buffer = disk_name(disk, part, buffer); > > put_disk(disk); > > + module_put(disk->fops->owner); > > } else { > > snprintf(buffer, BDEVNAME_SIZE, "unknown-block(%u,%u)", > > MAJOR(dev), MINOR(dev)); > > Yes, there's certainly a leak there. It's surprising that it hasn't been > noted before. > > But: > > const char *__bdevname(dev_t dev, char *buffer) > { > struct gendisk *disk; > int part; > > disk = get_gendisk(dev, &part); > if (disk) { > buffer = disk_name(disk, part, buffer); > put_disk(disk); > } else { > snprintf(buffer, BDEVNAME_SIZE, "unknown-block(%u,%u)", > MAJOR(dev), MINOR(dev)); > } > > get_gendisk() did an internal module_get() in kobj_lookup().
But if kobj_lookup() succeeds, module_put() is called before returning (actually it's always called, right?)
> It would seem to be logical that the module_put() should be in > kobject_put(), called from put_disk(). But surely if we made that > change 1000 things would explode.
I don't see how that would fix anything, as the module reference is not held after kobj_lookup() returns. Or am I missing something here?
thanks,
greg k-h - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |