linux-kernel - issues with 8ce352818820 ("pidfs: check for valid ioctl commands") and cdda1f26e74b ("pidfd: add ioctl to retrieve pid 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>] [day] [month] [year] [list]

Message-ID: <CAG48ez2K9A5GwtgqO31u9ZL292we8ZwAA=TJwwEv7wRuJ3j4Lw@mail.gmail.com>
Date: Wed, 29 Jan 2025 00:30:07 +0100
From: Jann Horn <jannh@...gle.com>
To: Christian Brauner <brauner@...nel.org>, Luca Boccassi <luca.boccassi@...il.com>
Cc: kernel list <linux-kernel@...r.kernel.org>
Subject: issues with 8ce352818820 ("pidfs: check for valid ioctl commands")
 and cdda1f26e74b ("pidfd: add ioctl to retrieve pid info")

Hi!

I think commit cdda1f26e74b ("pidfd: add ioctl to retrieve pid info")
introduced a small issue: The ioctl handler does not look at the
"type" part of the ioctl command, so something like ioctl(pidfd,
11+(64<<16), buf) reaches the pidfd_info() path. That could be a
problem because it can cause confusion about the semantics of the
ioctl call: If a daemon receives some random file descriptor from a
(potentially less privileged) client and expects the FD to be of some
specific type, it might call ioctl() on this FD with some
type-specific command and expect the call to fail if the FD is of the
wrong type; but due to the missing type check, the kernel instead
performs some action that userspace didn't expect. For example,
ioctl(fd, PERF_EVENT_IOC_MODIFY_ATTRIBUTES, ...) would _almost_ be
able to reach pidfd_info(), except that that ioctl is for some reason
defined as
_IOW('$', 11, struct perf_event_attr *)
and not
_IOW('$', 11, struct perf_event_attr)
.
This is the current state in 6.13 stable.

I was going to suggest backporting 8ce352818820 ("pidfs: check for
valid ioctl commands") to 6.13 as a fix, since that checks the ioctl
command more strictly; but then I realized that 8ce352818820 is also a
little wrong. "struct pidfd_info" seems to be deliberately designed to
be extensible, but if it is ever extended, the check in
pidfs_ioctl_valid() will need to be rewritten to avoid breaking old
userspace. Worse, newer userspace that uses an extended struct would
be rejected by kernels that contain commit 8ce352818820, so we should
probably ensure that the check introduced in 8ce352818820 is relaxed
before 6.14 is released.