[PATCH] pidfs: improve ioctl handling

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

 



Pidfs supports extensible and non-extensible ioctls. The extensible
ioctls need to check for the ioctl number itself not just the ioctl
command otherwise both backward- and forward compatibility are broken.

The pidfs ioctl handler also needs to look at the type of the ioctl
command to guard against cases where "[...] 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."
(cf. [1]]

Reported-by: Jann Horn <jannh@xxxxxxxxxx>
Cc: stable@xxxxxxxxxxxxxxx # v6.13
Fixes: https://lore.kernel.org/r/CAG48ez2K9A5GwtgqO31u9ZL292we8ZwAA=TJwwEv7wRuJ3j4Lw@xxxxxxxxxxxxxx [1]
Signed-off-by: Christian Brauner <brauner@xxxxxxxxxx>
---
Hey,

Jann reported that the pidfs extensible ioctl checking has two issues:

(1) We check for the ioctl command, not the number breaking forward and
    backward compatibility.

(2) We don't verify the type encoded in the ioctl to guard against
    ioctl number collisions.

This adresses both.

Greg, when this patch (or a version thereof) lands upstream then it
needs to please be backported to v6.13 together with the already
upstream commit 8ce352818820 ("pidfs: check for valid ioctl commands").

Christian
---
 fs/pidfs.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/fs/pidfs.c b/fs/pidfs.c
index 049352f973de..63f9699ebac3 100644
--- a/fs/pidfs.c
+++ b/fs/pidfs.c
@@ -287,7 +287,6 @@ static bool pidfs_ioctl_valid(unsigned int cmd)
 	switch (cmd) {
 	case FS_IOC_GETVERSION:
 	case PIDFD_GET_CGROUP_NAMESPACE:
-	case PIDFD_GET_INFO:
 	case PIDFD_GET_IPC_NAMESPACE:
 	case PIDFD_GET_MNT_NAMESPACE:
 	case PIDFD_GET_NET_NAMESPACE:
@@ -300,6 +299,17 @@ static bool pidfs_ioctl_valid(unsigned int cmd)
 		return true;
 	}
 
+	/* Extensible ioctls require some more careful checks. */
+	switch (_IOC_NR(cmd)) {
+	case _IOC_NR(PIDFD_GET_INFO):
+		/*
+		 * Try to prevent performing a pidfd ioctl when someone
+		 * 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 false;
 }
 

---
base-commit: 6470d2c6d4233a781c67f842d3c066bf1cfa4fdc
change-id: 20250204-work-pidfs-ioctl-6ef79a65e36b





[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux