| Message ID | 20240305032132.548958-1-lilingfeng@huaweicloud.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | block: move capacity validation to blkpg_do_ioctl() | expand |
On 3/5/24 12:21, Li Lingfeng wrote: > From: Li Lingfeng <lilingfeng3@huawei.com> > > 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@huawei.com> > --- > 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 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. > if (!disk_live(disk)) { > ret = -ENXIO; > goto out;
在 2024/3/5 12:30, Damien Le Moal 写道: > On 3/5/24 12:21, Li Lingfeng wrote: >> From: Li Lingfeng <lilingfeng3@huawei.com> >> >> 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@huawei.com> >> --- >> 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;
On 3/5/24 15:24, Li Lingfeng wrote: > > 在 2024/3/5 12:30, Damien Le Moal 写道: >> On 3/5/24 12:21, Li Lingfeng wrote: >>> From: Li Lingfeng <lilingfeng3@huawei.com> >>> >>> 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@huawei.com> >>> --- >>> 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. Oh ! My bad ! I got confused with blk_add_partition() which is the function adding partitions when a new disk is scanned. So yes, your patch is fine. My apologies for the noise. >> 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; > >
On 3/5/24 12:21, Li Lingfeng wrote: > From: Li Lingfeng <lilingfeng3@huawei.com> > > 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@huawei.com> Looks good. Reviewed-by: Damien Le Moal <dlemoal@kernel.org> But I wonder if this needs a Fixes: 6d4e80db4ebe ("block: add capacity validation in bdev_add_partition()") tag and Cc-stable...
Looks good:
Reviewed-by: Christoph Hellwig <hch@lst.de>
On Tue, 05 Mar 2024 11:21:32 +0800, Li Lingfeng wrote: > 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. > > > [...] Applied, thanks! [1/1] block: move capacity validation to blkpg_do_ioctl() commit: b9355185d2aeef11f6e365dd98def82212637936 Best regards,
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; - } - if (!disk_live(disk)) { ret = -ENXIO; goto out;