[PATCH 0/2] nsfs: validate ioctls

[PATCH 0/2] nsfs: validate ioctls

[Christian Brauner]

This series ensures that nsfs protects against ioctl overloading.

Signed-off-by: Christian Brauner <brauner@kernel.org>
---
Christian Brauner (2):
      nsfs: validate ioctls
      selftests/nsfs: add ioctl validation tests

 fs/nsfs.c                                          | 32 +++++++++++++++++++++-
 .../selftests/filesystems/nsfs/iterate_mntns.c     | 14 ++++++++++
 2 files changed, 45 insertions(+), 1 deletion(-)
---
base-commit: 2014c95afecee3e76ca4a56956a936e23283f05b
change-id: 20250219-work-nsfs-c72d880d9c3e

[PATCH 1/2] nsfs: validate ioctls

[Christian Brauner]

Nsfs supports extensible and non-extensible ioctls. Validate both types
to prevent confusion.

Signed-off-by: Christian Brauner <brauner@kernel.org>
---
 fs/nsfs.c | 32 +++++++++++++++++++++++++++++++-
 1 file changed, 31 insertions(+), 1 deletion(-)

diff --git a/fs/nsfs.c b/fs/nsfs.c
index 663f8656158d..1ab705bb9386 100644
--- a/fs/nsfs.c
+++ b/fs/nsfs.c
@@ -152,19 +152,49 @@ static int copy_ns_info_to_user(const struct mnt_namespace *mnt_ns,
 	return 0;
 }
 
+static bool nsfs_ioctl_valid(unsigned int cmd)
+{
+	switch (cmd) {
+	case NS_GET_USERNS:
+	case NS_GET_PARENT:
+	case NS_GET_NSTYPE:
+	case NS_GET_OWNER_UID:
+	case NS_GET_MNTNS_ID:
+	case NS_GET_PID_FROM_PIDNS:
+	case NS_GET_TGID_FROM_PIDNS:
+	case NS_GET_PID_IN_PIDNS:
+	case NS_GET_TGID_IN_PIDNS:
+		return (_IOC_TYPE(cmd) == _IOC_TYPE(cmd));
+	}
+
+	/* Extensible ioctls require some extra handling. */
+	switch (_IOC_NR(cmd)) {
+	case _IOC_NR(NS_MNT_GET_INFO):
+	case _IOC_NR(NS_MNT_GET_NEXT):
+	case _IOC_NR(NS_MNT_GET_PREV):
+		return (_IOC_TYPE(cmd) == _IOC_TYPE(cmd));
+	}
+
+	return false;
+}
+
 static long ns_ioctl(struct file *filp, unsigned int ioctl,
 			unsigned long arg)
 {
 	struct user_namespace *user_ns;
 	struct pid_namespace *pid_ns;
 	struct task_struct *tsk;
-	struct ns_common *ns = get_proc_ns(file_inode(filp));
+	struct ns_common *ns;
 	struct mnt_namespace *mnt_ns;
 	bool previous = false;
 	uid_t __user *argp;
 	uid_t uid;
 	int ret;
 
+	if (!nsfs_ioctl_valid(ioctl))
+		return -ENOIOCTLCMD;
+
+	ns = get_proc_ns(file_inode(filp));
 	switch (ioctl) {
 	case NS_GET_USERNS:
 		return open_related_ns(ns, ns_get_owner);

-- 
2.47.2

[PATCH 2/2] selftests/nsfs: add ioctl validation tests

[Christian Brauner]

Add simple tests to validate that non-nsfs ioctls are rejected.

Signed-off-by: Christian Brauner <brauner@kernel.org>
---
 tools/testing/selftests/filesystems/nsfs/iterate_mntns.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/tools/testing/selftests/filesystems/nsfs/iterate_mntns.c b/tools/testing/selftests/filesystems/nsfs/iterate_mntns.c
index 457cf76f3c5f..a3d8015897e9 100644
--- a/tools/testing/selftests/filesystems/nsfs/iterate_mntns.c
+++ b/tools/testing/selftests/filesystems/nsfs/iterate_mntns.c
@@ -3,6 +3,8 @@
 
 #define _GNU_SOURCE
 #include <fcntl.h>
+#include <linux/auto_dev-ioctl.h>
+#include <linux/errno.h>
 #include <sched.h>
 #include <stdio.h>
 #include <string.h>
@@ -146,4 +148,16 @@ TEST_F(iterate_mount_namespaces, iterate_backward)
 	}
 }
 
+TEST_F(iterate_mount_namespaces, nfs_valid_ioctl)
+{
+	ASSERT_NE(ioctl(self->fd_mnt_ns[0], AUTOFS_DEV_IOCTL_OPENMOUNT, NULL), 0);
+	ASSERT_EQ(errno, ENOTTY);
+
+	ASSERT_NE(ioctl(self->fd_mnt_ns[0], AUTOFS_DEV_IOCTL_CLOSEMOUNT, NULL), 0);
+	ASSERT_EQ(errno, ENOTTY);
+
+	ASSERT_NE(ioctl(self->fd_mnt_ns[0], AUTOFS_DEV_IOCTL_READY, NULL), 0);
+	ASSERT_EQ(errno, ENOTTY);
+}
+
 TEST_HARNESS_MAIN

-- 
2.47.2

Re: [PATCH 0/2] nsfs: validate ioctls

[Jeff Layton]

On Wed, 2025-02-19 at 17:40 +0100, Christian Brauner wrote:
> This series ensures that nsfs protects against ioctl overloading.
> 
> Signed-off-by: Christian Brauner <brauner@kernel.org>
> ---
> Christian Brauner (2):
>       nsfs: validate ioctls
>       selftests/nsfs: add ioctl validation tests
> 
>  fs/nsfs.c                                          | 32 +++++++++++++++++++++-
>  .../selftests/filesystems/nsfs/iterate_mntns.c     | 14 ++++++++++
>  2 files changed, 45 insertions(+), 1 deletion(-)
> ---
> base-commit: 2014c95afecee3e76ca4a56956a936e23283f05b
> change-id: 20250219-work-nsfs-c72d880d9c3e
> 

Looks like a reasonable thing to do.

Reviewed-by: Jeff Layton <jlayton@kernel.org>