Received-SPF: pass (google.com: domain of linux-kernel+bounces-27432-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Subject: Re: PROBLEM: BLKPG_DEL_PARTITION with GENHD_FL_NO_PART used to return
 ENXIO, now returns EINVAL
To: Yu Kuai <yukuai1@huaweicloud.com>, Christoph Hellwig <hch@infradead.org>,
 Allison Karlitskaya <allison.karlitskaya@redhat.com>
Cc: linux-kernel@vger.kernel.org, linux-block@vger.kernel.org,
 Jens Axboe <axboe@kernel.dk>, "yukuai (C)" <yukuai3@huawei.com>
References: <CAOYeF9VsmqKMcQjo1k6YkGNujwN-nzfxY17N3F-CMikE1tYp+w@mail.gmail.com>
 <ZaZesiKpbMEiiTrf@infradead.org>
 <210deda9-5439-244a-0ce2-af9dc8e5d7fe@huaweicloud.com>
From: Yu Kuai <yukuai1@huaweicloud.com>
Message-ID: <592625f7-36d7-02e0-2ee6-8211334aa0f9@huaweicloud.com>
Date: Tue, 16 Jan 2024 21:46:21 +0800
User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:60.0) Gecko/20100101
 Thunderbird/60.8.0
Precedence: bulk
MIME-Version: 1.0
In-Reply-To: <210deda9-5439-244a-0ce2-af9dc8e5d7fe@huaweicloud.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit

Hi,

在 2024/01/16 21:23, Yu Kuai 写道:
> Hi, Christoph
> 
> 在 2024/01/16 18:47, Christoph Hellwig 写道:
>> Hi Allison,
>>
>> please try this minimal fix.  I need to double check if we historically
>> returned ENXIO or EINVAL for adding / resizing partitions, which would
>> make things more complicated.  Or maybe you already have data for that
>> at hand?
>>
>> diff --git a/block/ioctl.c b/block/ioctl.c
>> index 9c73a763ef8838..f2028e39767821 100644
>> --- a/block/ioctl.c
>> +++ b/block/ioctl.c
>> @@ -21,7 +21,7 @@ static int blkpg_do_ioctl(struct block_device *bdev,
>>       sector_t start, length;
>>       if (disk->flags & GENHD_FL_NO_PART)
>> -        return -EINVAL;
>> +        return -ENXIO;
> 
> I think this might not be a proper fix, the reason if that before this
> condition is added, -ENXIO is returned from bdev_del_partition(). And
> there are also some other error number like -EACCES,-EFAULT following,
> so this change will still make changes for user in other cases.

Please ignore the patch from last email. Sorry for the noise...
bdev_resize_partition() will also return -ENXIO if partition does't
exist. So the right patch should be following:

diff --git a/block/ioctl.c b/block/ioctl.c
index 4160f4e6bd5b..ba8d44fa7e02 100644
--- a/block/ioctl.c
+++ b/block/ioctl.c
@@ -20,8 +20,6 @@ static int blkpg_do_ioctl(struct block_device *bdev,
         struct blkpg_partition p;
         long long start, length;

-       if (disk->flags & GENHD_FL_NO_PART)
-               return -EINVAL;
         if (!capable(CAP_SYS_ADMIN))
                 return -EACCES;
         if (copy_from_user(&p, upart, sizeof(struct blkpg_partition)))
diff --git a/block/partitions/core.c b/block/partitions/core.c
index f47ffcfdfcec..f14602022c5e 100644
--- a/block/partitions/core.c
+++ b/block/partitions/core.c
@@ -447,6 +447,11 @@ int bdev_add_partition(struct gendisk *disk, int 
partno, sector_t start,
                 goto out;
         }

+       if (disk->flags & GENHD_FL_NO_PART) {
+               ret = -EINVAL;
+               goto out;
+       }
+
         if (partition_overlaps(disk, start, length, -1)) {
                 ret = -EBUSY;
                 goto out;

Thanks,
Kuai
> 
> How about following patch?
> 
> diff --git a/block/ioctl.c b/block/ioctl.c
> index 4160f4e6bd5b..ec012cf910dc 100644
> --- a/block/ioctl.c
> +++ b/block/ioctl.c
> @@ -20,8 +20,6 @@ static int blkpg_do_ioctl(struct block_device *bdev,
>          struct blkpg_partition p;
>          long long start, length;
> 
> -       if (disk->flags & GENHD_FL_NO_PART)
> -               return -EINVAL;
>          if (!capable(CAP_SYS_ADMIN))
>                  return -EACCES;
>          if (copy_from_user(&p, upart, sizeof(struct blkpg_partition)))
> @@ -38,6 +36,9 @@ static int blkpg_do_ioctl(struct block_device *bdev,
>          start = p.start >> SECTOR_SHIFT;
>          length = p.length >> SECTOR_SHIFT;
> 
> +       if (disk->flags & GENHD_FL_NO_PART)
> +               return -EINVAL;
> +
>          switch (op) {
>          case BLKPG_ADD_PARTITION:
>                  /* check if partition is aligned to blocksize */
> 
> Thanks,
> Kuai
> 
> 
>>       if (!capable(CAP_SYS_ADMIN))
>>           return -EACCES;
>>       if (copy_from_user(&p, upart, sizeof(struct blkpg_partition)))
>>
>> .
>>
> 
> .
>