linux-kernel - Re: 6.18.0-rc1: LTP syscalls ioctl_pidfd05: TFAIL: ioctl(pidfd, PIDFD_GET_INFO_SHORT, info

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: <lguqncbotw2cu2nfaf6hwgip6wtrmeg2azvyeht7l56itlomy5@uccupuql3let>
Date: Tue, 21 Oct 2025 22:48:49 +0200
From: Jan Kara <jack@...e.cz>
To: Christian Brauner <brauner@...nel.org>
Cc: Jan Kara <jack@...e.cz>, Cyril Hrubis <chrubis@...e.cz>, 
	Naresh Kamboju <naresh.kamboju@...aro.org>, open list <linux-kernel@...r.kernel.org>, 
	linux-fsdevel@...r.kernel.org, lkft-triage@...ts.linaro.org, 
	Linux Regressions <regressions@...ts.linux.dev>, LTP List <ltp@...ts.linux.it>, 
	Andrey Albershteyn <aalbersh@...nel.org>, Arnd Bergmann <arnd@...db.de>, 
	Alexander Viro <viro@...iv.linux.org.uk>, Dan Carpenter <dan.carpenter@...aro.org>, 
	Anders Roxell <anders.roxell@...aro.org>, Ben Copeland <benjamin.copeland@...aro.org>, 
	Petr Vorel <pvorel@...e.cz>, Andrea Cervesato <andrea.cervesato@...e.com>
Subject: Re: 6.18.0-rc1: LTP syscalls ioctl_pidfd05: TFAIL: ioctl(pidfd,
 PIDFD_GET_INFO_SHORT, info_invalid) expected EINVAL: ENOTTY (25)

On Tue 21-10-25 15:21:08, Christian Brauner wrote:
> On Fri, Oct 17, 2025 at 02:43:14PM +0200, Jan Kara wrote:
> > On Fri 17-10-25 11:40:41, Cyril Hrubis wrote:
> > > Hi!
> > > > > ## Test error log
> > > > > tst_buffers.c:57: TINFO: Test is using guarded buffers
> > > > > tst_test.c:2021: TINFO: LTP version: 20250930
> > > > > tst_test.c:2024: TINFO: Tested kernel: 6.18.0-rc1 #1 SMP PREEMPT
> > > > > @1760657272 aarch64
> > > > > tst_kconfig.c:88: TINFO: Parsing kernel config '/proc/config.gz'
> > > > > tst_kconfig.c:676: TINFO: CONFIG_TRACE_IRQFLAGS kernel option detected
> > > > > which might slow the execution
> > > > > tst_test.c:1842: TINFO: Overall timeout per run is 0h 21m 36s
> > > > > ioctl_pidfd05.c:45: TPASS: ioctl(pidfd, PIDFD_GET_INFO, NULL) : EINVAL (22)
> > > > > ioctl_pidfd05.c:46: TFAIL: ioctl(pidfd, PIDFD_GET_INFO_SHORT,
> > > > > info_invalid) expected EINVAL: ENOTTY (25)
> > > 
> > > Looking closely this is a different problem.
> > > 
> > > What we do in the test is that we pass PIDFD_IOCTL_INFO whith invalid
> > > size with:
> > > 
> > > struct pidfd_info_invalid {
> > >         uint32_t dummy;
> > > };
> > > 
> > > #define PIDFD_GET_INFO_SHORT _IOWR(PIDFS_IOCTL_MAGIC, 11, struct pidfd_info_invalid)
> > > 
> > > 
> > > And we expect to hit:
> > > 
> > >         if (usize < PIDFD_INFO_SIZE_VER0)
> > >                 return -EINVAL; /* First version, no smaller struct possible */
> > > 
> > > in fs/pidfs.c
> > > 
> > > 
> > > And apparently the return value was changed in:
> > > 
> > > commit 3c17001b21b9f168c957ced9384abe969019b609
> > > Author: Christian Brauner <brauner@...nel.org>
> > > Date:   Fri Sep 12 13:52:24 2025 +0200
> > > 
> > >     pidfs: validate extensible ioctls
> > >     
> > >     Validate extensible ioctls stricter than we do now.
> > >     
> > >     Reviewed-by: Aleksa Sarai <cyphar@...har.com>
> > >     Reviewed-by: Jan Kara <jack@...e.cz>
> > >     Signed-off-by: Christian Brauner <brauner@...nel.org>
> > > 
> > > diff --git a/fs/pidfs.c b/fs/pidfs.c
> > > index edc35522d75c..0a5083b9cce5 100644
> > > --- a/fs/pidfs.c
> > > +++ b/fs/pidfs.c
> > > @@ -440,7 +440,7 @@ static bool pidfs_ioctl_valid(unsigned int cmd)
> > >                  * erronously mistook the file descriptor for a pidfd.
> > >                  * This is not perfect but will catch most cases.
> > >                  */
> > > -               return (_IOC_TYPE(cmd) == _IOC_TYPE(PIDFD_GET_INFO));
> > > +               return extensible_ioctl_valid(cmd, PIDFD_GET_INFO, PIDFD_INFO_SIZE_VER0);
> > >         }
> > >  
> > >         return false;
> > > 
> > > 
> > > So kernel has changed error it returns, if this is a regression or not
> > > is for kernel developers to decide.
> > 
> > Yes, it's mostly a question to Christian whether if passed size for
> > extensible ioctl is smaller than minimal, we should be returning
> > ENOIOCTLCMD or EINVAL. I think EINVAL would make more sense but Christian
> > is our "extensible ioctl expert" :).
> 
> You're asking difficult questions actually. :D
> I think it would be completely fine to return EINVAL in this case.
> But traditionally ENOTTY has been taken to mean that this is not a
> supported ioctl. This translation is done by the VFS layer itself iirc.

Now the translation is done by VFS, I agree. But in the past (when the LTP
test was written) extensible ioctl with too small structure passed the
initial checks, only later we found out the data is too short and returned
EINVAL for that case. I *think* we are fine with just adjusting the test to
accept the new world order but wanted your opinion what are the chances of
some real userspace finding the old behavior useful or otherwise depending
on it.

								Honza
-- 
Jan Kara <jack@...e.com>
SUSE Labs, CR