Re: [PATCH] [v2] block: fix FS_IOC_GETLBMD_CAP parsing in blkdev_common_ioctl()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, 11 Jul 2025 at 10:47, Arnd Bergmann <arnd@xxxxxxxxxx> wrote:
>
> From: Arnd Bergmann <arnd@xxxxxxxx>
>
> Anders and Naresh found that the addition of the FS_IOC_GETLBMD_CAP
> handling in the blockdev ioctl handler breaks all ioctls with
> _IOC_NR==2, as the new command is not added to the switch but only
> a few of the command bits are check.
>
> Move the check into the blk_get_meta_cap() function itself and make
> it return -ENOIOCTLCMD for any unsupported command code, including
> those with a smaller size that previously returned -EINVAL.
>
> For consistency this also drops the check for NULL 'arg' that
> is really useless, as any invalid pointer should return -EFAULT.
>
> Fixes: 9eb22f7fedfc ("fs: add ioctl to query metadata and protection info capabilities")
> Link: https://lore.kernel.org/all/CA+G9fYvk9HHE5UJ7cdJHTcY6P5JKnp+_e+sdC5U-ZQFTP9_hqQ@xxxxxxxxxxxxxx/
> Reported-by: Naresh Kamboju <naresh.kamboju@xxxxxxxxxx>
> Cc: Anders Roxell <anders.roxell@xxxxxxxxxx>
> Cc: Naresh Kamboju <naresh.kamboju@xxxxxxxxxx>
> Signed-off-by: Arnd Bergmann <arnd@xxxxxxxx>

Tested-by: Anders Roxell <anders.roxell@xxxxxxxxxx>

Tested it in qemu-aarch64, it now ran the ltp-smoke test fine.

Cheers,
Anders

> ---
> v2: add the check in blk-integrity.c instead of ioctl.c
>
> I've left out the maximum-size check this time, as there was no
> consensus on whether there should be one, or what value.
>
> We still need to come up with a better way of handling these in
> general, for now the patch just addresses the immediate regression
> that Naresh found.
>
> I have also sent a handful of patches for other drivers that have
> variations of the same bug.
> ---
>  block/blk-integrity.c | 10 ++++++----
>  block/ioctl.c         |  6 ++++--
>  2 files changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/block/blk-integrity.c b/block/blk-integrity.c
> index 9d9dc9c32083..61a79e19c78f 100644
> --- a/block/blk-integrity.c
> +++ b/block/blk-integrity.c
> @@ -62,10 +62,12 @@ int blk_get_meta_cap(struct block_device *bdev, unsigned int cmd,
>         struct logical_block_metadata_cap meta_cap = {};
>         size_t usize = _IOC_SIZE(cmd);
>
> -       if (!argp)
> -               return -EINVAL;
> -       if (usize < LBMD_SIZE_VER0)
> -               return -EINVAL;
> +       if (_IOC_DIR(cmd)  != _IOC_DIR(FS_IOC_GETLBMD_CAP) ||
> +           _IOC_TYPE(cmd) != _IOC_TYPE(FS_IOC_GETLBMD_CAP) ||
> +           _IOC_NR(cmd)   != _IOC_NR(FS_IOC_GETLBMD_CAP) ||
> +           _IOC_SIZE(cmd) < LBMD_SIZE_VER0)
> +               return -ENOIOCTLCMD;
> +
>         if (!bi)
>                 goto out;
>
> diff --git a/block/ioctl.c b/block/ioctl.c
> index 9ad403733e19..af2e22e5533c 100644
> --- a/block/ioctl.c
> +++ b/block/ioctl.c
> @@ -566,9 +566,11 @@ static int blkdev_common_ioctl(struct block_device *bdev, blk_mode_t mode,
>                                void __user *argp)
>  {
>         unsigned int max_sectors;
> +       int ret;
>
> -       if (_IOC_NR(cmd) == _IOC_NR(FS_IOC_GETLBMD_CAP))
> -               return blk_get_meta_cap(bdev, cmd, argp);
> +       ret = blk_get_meta_cap(bdev, cmd, argp);
> +       if (ret != -ENOIOCTLCMD)
> +               return ret;
>
>         switch (cmd) {
>         case BLKFLSBUF:
> --
> 2.39.5
>




[Index of Archives]     [Linux RAID]     [Linux SCSI]     [Linux ATA RAID]     [IDE]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Device Mapper]

  Powered by Linux