Re: PROBLEM: BLKPG_DEL_PARTITION with GENHD_FL_NO_PART used to return ENXIO, now returns EINVAL

[Yu Kuai]

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)))
> >
> > .
>
> .