linux-kernel - Re: [PATCH] [v2] block: fix FS_IOC_GETLBMD_CAP parsing in blkdev_common

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <d556ddfd-36f0-4ae3-aac4-9dc0f903d7b1@samsung.com>
Date: Mon, 28 Jul 2025 15:39:39 +0200
From: Marek Szyprowski <m.szyprowski@...sung.com>
To: Arnd Bergmann <arnd@...nel.org>, Anuj Gupta <anuj20.g@...sung.com>,
	"Martin K. Petersen" <martin.petersen@...cle.com>, Kanchan Joshi
	<joshi.k@...sung.com>, Christian Brauner <brauner@...nel.org>
Cc: Christoph Hellwig <hch@...radead.org>, Arnd Bergmann <arnd@...db.de>,
	Naresh Kamboju <naresh.kamboju@...aro.org>, Anders Roxell
	<anders.roxell@...aro.org>, Jens Axboe <axboe@...nel.dk>, Keith Busch
	<kbusch@...nel.org>, Caleb Sander Mateos <csander@...estorage.com>, Pavel
	Begunkov <asml.silence@...il.com>, Alexey Dobriyan <adobriyan@...il.com>,
	"Darrick J. Wong" <djwong@...nel.org>, linux-block@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] [v2] block: fix FS_IOC_GETLBMD_CAP parsing in
 blkdev_common_ioctl()

On 11.07.2025 10:46, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@...db.de>
>
> 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@mail.gmail.com/
> Reported-by: Naresh Kamboju <naresh.kamboju@...aro.org>
> Cc: Anders Roxell <anders.roxell@...aro.org>
> Cc: Naresh Kamboju <naresh.kamboju@...aro.org>
> Signed-off-by: Arnd Bergmann <arnd@...db.de>
> ---
> 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.
> ---

In my tests I've found that this patch, merged as commit 42b0ef01e6b5 
("block: fix FS_IOC_GETLBMD_CAP parsing in blkdev_common_ioctl()"), 
breaks udev operation on some of my test boards - no /dev/disk/* entries 
and directories are created. Reverting $subject on top of next-20250728 
fixes/hides this problem. I suspect that another corner case is missing 
in the checks. I will try to investigate this a bit more later, probably 
tomorrow.


>   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:

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland