Re: [PATCH] CDRW packet writing support for 2.6.7-bk13

[Andrew Morton]

Peter Osterlund <petero2@xxxxxxxxx> 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@xxxxxxxxx>
> 
> ---
> 
>  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().  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.

Greg, Al?

-
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/