lkml.org 
[lkml]   [2004]   [Jul]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [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/

    \
     
     \ /
      Last update: 2005-03-22 14:04    [W:0.030 / U:0.364 seconds]
    ©2003-2017 Jasper Spaans. hosted at Digital OceanAdvertise on this site