Thread (4 messages) 4 messages, 2 authors, 2020-01-14

Re: [PATCH V2] brd: check parameter validation before register_blkdev func

From: Zhiqiang Liu <hidden>
Date: 2020-01-14 11:39:11
Also in: lkml

On 2020/1/14 17:45, Ming Lei wrote:
On Tue, Jan 14, 2020 at 05:16:57PM +0800, Ming Lei wrote:
quoted
On Mon, Jan 13, 2020 at 09:43:23PM +0800, Zhiqiang Liu wrote:
quoted
In brd_init func, rd_nr num of brd_device are firstly allocated
and add in brd_devices, then brd_devices are traversed to add each
brd_device by calling add_disk func. When allocating brd_device,
the disk->first_minor is set to i * max_part, if rd_nr * max_part
is larger than MINORMASK, two different brd_device may have the same
devt, then only one of them can be successfully added.
It is just because disk->first_minor is >= 0x100000, then same dev_t
can be allocated in blk_alloc_devt().

	MKDEV(disk->major, disk->first_minor + part->partno)

But block layer does support extended dynamic devt allocation, and brd
sets flag of GENHD_FL_EXT_DEVT too.

So I think the correct fix is to fallback to extended dynamic allocation
when running out of consecutive minor space.

How about the following approach?

And of course, ext devt allocation may fail too, but that is another
generic un-solved issue: error handling isn't done for adding disk.
diff --git a/drivers/block/brd.c b/drivers/block/brd.c
index a8730cc4db10..9aa7ce7c9abf 100644
--- a/drivers/block/brd.c
+++ b/drivers/block/brd.c
@@ -398,7 +398,16 @@ static struct brd_device *brd_alloc(int i)
 	if (!disk)
 		goto out_free_queue;
 	disk->major		= RAMDISK_MAJOR;
-	disk->first_minor	= i * max_part;
+
+	/*
+	 * Clear .minors when running out of consecutive minor space since
+	 * GENHD_FL_EXT_DEVT is set, and we can allocate from extended devt
+	 */
+	if ((i * disk->minors) & ~MINORMASK)
+		disk->minors = 0;
+	else
+		disk->first_minor	= i * disk->minors;
+
 	disk->fops		= &brd_fops;
 	disk->private_data	= brd;
 	disk->flags		= GENHD_FL_EXT_DEVT;
But still suggest to limit 'max_part' <= 256, and the name is actually
misleading, which just reserves consecutive minors.

However, I don't think it is a good idea to add limit on device number.
Thanks for your patient replyI will resend the v3 patch as your suggestion.
Changes in v3:
	1)clear .minors when running out of consecutive minor space in brd_alloc.
	2)remove limit of rd_nr

Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help