Received-SPF: pass (google.com: domain of linux-kernel+bounces-91656-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Message-ID: <474e78e7-9056-4b74-9ce2-592b8ee9411c@kernel.org>
Date: Tue, 5 Mar 2024 13:30:45 +0900
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH] block: move capacity validation to blkpg_do_ioctl()
Content-Language: en-US
To: Li Lingfeng <lilingfeng@huaweicloud.com>, axboe@kernel.dk,
 min15.li@samsung.com
Cc: linux-block@vger.kernel.org, linux-kernel@vger.kernel.org, hch@lst.de,
 yangerkun@huawei.com, yukuai1@huaweicloud.com, houtao1@huawei.com,
 yi.zhang@huawei.com
References: <20240305032132.548958-1-lilingfeng@huaweicloud.com>
From: Damien Le Moal <dlemoal@kernel.org>
Organization: Western Digital Research
In-Reply-To: <20240305032132.548958-1-lilingfeng@huaweicloud.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit

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;

-- 
Damien Le Moal
Western Digital Research