linux-kernel - Re: [PATCH] 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] [thread-next>] [day] [month] [year] [list]

Message-Id: <290c17df-1bf2-45b8-b0c2-7a1865585d0a@app.fastmail.com>
Date: Wed, 09 Jul 2025 22:30:40 +0200
From: "Arnd Bergmann" <arnd@...db.de>
To: "Darrick J. Wong" <djwong@...nel.org>, "Arnd Bergmann" <arnd@...nel.org>
Cc: linux-fsdevel@...r.kernel.org, linux-block@...r.kernel.org,
 "Anuj Gupta" <anuj20.g@...sung.com>,
 "Martin K. Petersen" <martin.petersen@...cle.com>,
 "Kanchan Joshi" <joshi.k@...sung.com>, "LTP List" <ltp@...ts.linux.it>,
 "Dan Carpenter" <dan.carpenter@...aro.org>,
 "Benjamin Copeland" <benjamin.copeland@...aro.org>, rbm@...e.com,
 "Naresh Kamboju" <naresh.kamboju@...aro.org>,
 "Anders Roxell" <anders.roxell@...aro.org>, "Jens Axboe" <axboe@...nel.dk>,
 "Pavel Begunkov" <asml.silence@...il.com>,
 "Christian Brauner" <brauner@...nel.org>,
 "Alexey Dobriyan" <adobriyan@...il.com>,
 "Eric Biggers" <ebiggers@...gle.com>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] block: fix FS_IOC_GETLBMD_CAP parsing in blkdev_common_ioctl()

On Wed, Jul 9, 2025, at 20:27, Darrick J. Wong wrote:
> On Wed, Jul 09, 2025 at 08:10:14PM +0200, Arnd Bergmann wrote:

> though we probably want a helper or something to encapsulate those three
> comparisons to avoid the SOMETHING_SOMETHING part:
>
> #define IOC_DISPATCH(c) \
> 	((c) & ~(_IOC(0, 0, 0, _IOC_SIZE(_IOC_SIZEMASK))))
>
> 	switch (IOC_DISPATCH(cmd)) {
> 	case IOC_DISPATCH(FS_IOC_FSGETXATTR):
> 		return ioctl_fsgetxattr(filp, cmd, argp);
>
> Assuming that ioctl_fsgetxattr derives size from @cmd and rejects values
> that it doesn't like.  Hrm?

This may work in specific cases, but it adds a lot of complexity
and room for error if we try to do this in more places:

Ignoring the 'size' argument as above would mean that
each case now has to add an extra size check in each 'case',
which then defeats the entire purpose.

I should maybe dig out my notes for table-driver ioctl
handlers, if we want to improve the way that drivers define
their ioctl implementations, I'm sure there is some
infrastructure we can come up with that can help here,
but I don't think 'same as before but more macros' is the
answer.

joydev_ioctl_common() is an existing example doing something
like it and gets it right, while snd_compr_ioctl() is an
example that looks completely broken to me.

>> +	    _IOC_SIZE(cmd) >= LBMD_SIZE_VER0 &&
>> +	    _IOC_SIZE(cmd) <= _IOC_SIZE(FS_IOC_GETLBMD_CAP))
>
> blk_get_meta_cap already checks this.

I had thought about removing it there, but decided against that.
Maybe a better way would be to have the full check inside of 
blk_get_meta_cap() and use the -ENOIOCTLCMD return code
to keep the caller simple:

   switch(cmd) {
   ...
   default:
         break;
   }
   ret = blk_get_meta_cap(bdev, cmd, argp);
   if (ret != -ENOIOCTLCMD)
        return ret;
   ...
   return -ENOIOCTLCMD;

       Arnd