Re: [PATCH] block: move capacity validation to blkpg_do_ioctl()

[Li Lingfeng <lilingfeng@xxxxxxxxxxxxxxx>]

在 2024/3/5 12:30, Damien Le Moal 写道:
> On 3/5/24 12:21, Li Lingfeng wrote:
> > From: Li Lingfeng <lilingfeng3@xxxxxxxxxx>
> >
> > Commit 6d4e80db4ebe ("block: add capacity validation in
> > bdev_add_partition()") add check of partition's start and end sectors to
> > prevent exceeding the size of the disk when adding partitions. However,
> > there is still no check for resizing partitions now.
> > Move the check to blkpg_do_ioctl() to cover resizing partitions.
> >
> > Signed-off-by: Li Lingfeng <lilingfeng3@xxxxxxxxxx>
> > ---
> >   block/ioctl.c           |  9 ++++++++-
> >   block/partitions/core.c | 11 -----------
> >   2 files changed, 8 insertions(+), 12 deletions(-)
> >
> > diff --git a/block/ioctl.c b/block/ioctl.c
> > index 438f79c564cf..de0cc0d215c6 100644
> > --- a/block/ioctl.c
> > +++ b/block/ioctl.c
> > @@ -18,7 +18,7 @@ static int blkpg_do_ioctl(struct block_device *bdev,
> >   {
> >   	struct gendisk *disk = bdev->bd_disk;
> >   	struct blkpg_partition p;
> > -	sector_t start, length;
> > +	sector_t start, length, capacity, end;
> >   
> >   	if (!capable(CAP_SYS_ADMIN))
> >   		return -EACCES;
> > @@ -41,6 +41,13 @@ static int blkpg_do_ioctl(struct block_device *bdev,
> >   
> >   	start = p.start >> SECTOR_SHIFT;
> >   	length = p.length >> SECTOR_SHIFT;
> > +	capacity = get_capacity(disk);
> > +
> > +	if (check_add_overflow(start, length, &end))
> > +		return -EINVAL;
> > +
> > +	if (start >= capacity || end > capacity)
> > +		return -EINVAL;
> >   
> >   	switch (op) {
> >   	case BLKPG_ADD_PARTITION:
> > diff --git a/block/partitions/core.c b/block/partitions/core.c
> > index 5f5ed5c75f04..b11e88c82c8c 100644
> > --- a/block/partitions/core.c
> > +++ b/block/partitions/core.c
> > @@ -419,21 +419,10 @@ static bool partition_overlaps(struct gendisk *disk, sector_t start,
> >   int bdev_add_partition(struct gendisk *disk, int partno, sector_t start,
> >   		sector_t length)
> >   {
> > -	sector_t capacity = get_capacity(disk), end;
> >   	struct block_device *part;
> >   	int ret;
> >   
> >   	mutex_lock(&disk->open_mutex);
> > -	if (check_add_overflow(start, length, &end)) {
> > -		ret = -EINVAL;
> > -		goto out;
> > -	}
> > -
> > -	if (start >= capacity || end > capacity) {
> > -		ret = -EINVAL;
> > -		goto out;
> > -	}
> > -
> Why do you remove this ? The check will not be done when *existing* partitions
I'm sorry, I didn't quite understand your point. The function 
bdev_add_partition() is only called within blkpg_do_ioctl(). I simply 
moved the check to a different location, but did not remove it entirely.
> are added. To do the check when *creating* a partition, make this code a helper
> and call that helper function here and from blkpg_do_ioctl() as well.
Do you mean call the helper function in blk_add_partition() and 
blkpg_do_ioctl()?
> >   	if (!disk_live(disk)) {
> >   		ret = -ENXIO;
> >   		goto out;