[PATCH] block: fix leaking minors of hidden disks

[PATCH] block: fix leaking minors of hidden disks

[Christoph Hellwig]

The major/minor of a hidden gendisk is not propagated to the block
device because it is never registered using bdev_add.  But the lack of
bd_dev also causes the dynamic major minor number not to be freed.
Assign bd_dev manually to ensure the dynamic major minor gets freed.

Based on a patch by Keith Busch.

Fixes:  8ddcd653257c ("block: introduce GENHD_FL_HIDDEN")
Reported-by: Daniel Wagner <dwagner@suse.de>
Signed-off-by: Christoph Hellwig <hch@lst.de>
Tested-by: Daniel Wagner <dwagner@suse.de>
---
 block/genhd.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/block/genhd.c b/block/genhd.c
index 514395361d7c5..17b33c62423df 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -507,6 +507,13 @@ int __must_check device_add_disk(struct device *parent, struct gendisk *disk,
 		 */
 		dev_set_uevent_suppress(ddev, 0);
 		disk_uevent(disk, KOBJ_ADD);
+	} else {
+		/*
+		 * Even if the block_device for a hidden gendisk is not
+		 * registered, it needs to have a valid bd_dev so that the
+		 * freeing of the dynamic major works.
+		 */
+		disk->part0->bd_dev = MKDEV(disk->major, disk->first_minor);
 	}
 
 	disk_update_readahead(disk);
-- 
2.30.2

Re: [PATCH] block: fix leaking minors of hidden disks

[Keith Busch]

On Mon, Oct 10, 2022 at 3:19 PM Christoph Hellwig <hch@lst.de> wrote:
>
> The major/minor of a hidden gendisk is not propagated to the block
> device because it is never registered using bdev_add.  But the lack of
> bd_dev also causes the dynamic major minor number not to be freed.
> Assign bd_dev manually to ensure the dynamic major minor gets freed.

Looks good.

Reviewed-by: Keith Busch <kbusch@kernel.org>

Re: [PATCH] block: fix leaking minors of hidden disks

[Jens Axboe]

On Mon, 10 Oct 2022 15:18:57 +0200, Christoph Hellwig wrote:
> The major/minor of a hidden gendisk is not propagated to the block
> device because it is never registered using bdev_add.  But the lack of
> bd_dev also causes the dynamic major minor number not to be freed.
> Assign bd_dev manually to ensure the dynamic major minor gets freed.
> 
> Based on a patch by Keith Busch.
> 
> [...]

Applied, thanks!

[1/1] block: fix leaking minors of hidden disks
      commit: a0a6314ae774f8a5e52a599946aa2ad0db867b83

Best regards,
-- 
Jens Axboe

Re: [PATCH] block: fix leaking minors of hidden disks

[Hannes Reinecke]

On 10/10/22 15:18, Christoph Hellwig wrote:
> The major/minor of a hidden gendisk is not propagated to the block
> device because it is never registered using bdev_add.  But the lack of
> bd_dev also causes the dynamic major minor number not to be freed.
> Assign bd_dev manually to ensure the dynamic major minor gets freed.
> 
> Based on a patch by Keith Busch.
> 
> Fixes:  8ddcd653257c ("block: introduce GENHD_FL_HIDDEN")
> Reported-by: Daniel Wagner <dwagner@suse.de>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Tested-by: Daniel Wagner <dwagner@suse.de>
> ---
>   block/genhd.c | 7 +++++++
>   1 file changed, 7 insertions(+)
> 
> diff --git a/block/genhd.c b/block/genhd.c
> index 514395361d7c5..17b33c62423df 100644
> --- a/block/genhd.c
> +++ b/block/genhd.c
> @@ -507,6 +507,13 @@ int __must_check device_add_disk(struct device *parent, struct gendisk *disk,
>   		 */
>   		dev_set_uevent_suppress(ddev, 0);
>   		disk_uevent(disk, KOBJ_ADD);
> +	} else {
> +		/*
> +		 * Even if the block_device for a hidden gendisk is not
> +		 * registered, it needs to have a valid bd_dev so that the
> +		 * freeing of the dynamic major works.
> +		 */
> +		disk->part0->bd_dev = MKDEV(disk->major, disk->first_minor);
>   	}
>   
>   	disk_update_readahead(disk);

Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                Kernel Storage Architect
hare@suse.de                              +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Ivo Totev, Andrew
Myers, Andrew McDonald, Martje Boudien Moerman

Re: [PATCH] block: fix leaking minors of hidden disks

[Keith Busch]

On Fri, Oct 07, 2022 at 12:32:34PM -0700, Keith Busch wrote:
> +	if (disk->major == BLOCK_EXT_MAJOR)
> +		blk_free_ext_minor(disk->first_minor);
>  	iput(disk->part0->bd_inode);	/* frees the disk */
>  }

Oops, the above is missing the check for the hidden flag. V2 coming.

[PATCH] block: fix leaking minors of hidden disks

[Keith Busch]

From: Keith Busch <kbusch@kernel.org>

The major/minor of a hidden gendisk is not propagated to the block
device. This is required to suppress it from being visible. For these
disks, we need to handle freeing the dynamic minor directly when it's
released since bdev_free_inode() won't be able to.

Cc: Christoph Hellwig <hch@lst.de>
Reported-by: Daniel Wagner <dwagner@suse.de>
Signed-off-by: Keith Busch <kbusch@kernel.org>
---
 block/genhd.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/block/genhd.c b/block/genhd.c
index 514395361d7c..d46edf26320a 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -1166,6 +1166,8 @@ static void disk_release(struct device *dev)
 	if (test_bit(GD_ADDED, &disk->state) && disk->fops->free_disk)
 		disk->fops->free_disk(disk);
 
+	if (disk->major == BLOCK_EXT_MAJOR)
+		blk_free_ext_minor(disk->first_minor);
 	iput(disk->part0->bd_inode);	/* frees the disk */
 }
 
-- 
2.30.2