Unverified Commit 10047142 authored by Christian Brauner's avatar Christian Brauner
Browse files

Merge patch series "tighten nstree visibility checks"

Christian Brauner <brauner@kernel.org> says:

Listing various namespaces is currently only scoped on owning namespace.
We can make this more fine-grained so that we scope visibility even
tighter. To make it possible to change behavior restrict visibility for
now. This shouldn't be a big deal as there aren't actual large users out
there and paves the way to make this even cleaner in the future.

* patches from https://patch.msgid.link/20260226-work-visibility-fixes-v1-0-d2c2853313bd@kernel.org:
  selftests: fix mntns iteration selftests
  nstree: tighten permission checks for listing
  nsfs: tighten permission checks for handle opening
  nsfs: tighten permission checks for ns iteration ioctls

Link: https://patch.msgid.link/20260226-work-visibility-fixes-v1-0-d2c2853313bd@kernel.org


Signed-off-by: default avatarChristian Brauner <brauner@kernel.org>
parents a0b4c7a4 4c7b2ec2
Loading
Loading
Loading
Loading
+14 −1
Original line number Diff line number Diff line
@@ -199,6 +199,17 @@ static bool nsfs_ioctl_valid(unsigned int cmd)
	return false;
}

static bool may_use_nsfs_ioctl(unsigned int cmd)
{
	switch (_IOC_NR(cmd)) {
	case _IOC_NR(NS_MNT_GET_NEXT):
		fallthrough;
	case _IOC_NR(NS_MNT_GET_PREV):
		return may_see_all_namespaces();
	}
	return true;
}

static long ns_ioctl(struct file *filp, unsigned int ioctl,
			unsigned long arg)
{
@@ -214,6 +225,8 @@ static long ns_ioctl(struct file *filp, unsigned int ioctl,

	if (!nsfs_ioctl_valid(ioctl))
		return -ENOIOCTLCMD;
	if (!may_use_nsfs_ioctl(ioctl))
		return -EPERM;

	ns = get_proc_ns(file_inode(filp));
	switch (ioctl) {
@@ -614,7 +627,7 @@ static struct dentry *nsfs_fh_to_dentry(struct super_block *sb, struct fid *fh,
		return ERR_PTR(-EOPNOTSUPP);
	}

	if (owning_ns && !ns_capable(owning_ns, CAP_SYS_ADMIN)) {
	if (owning_ns && !may_see_all_namespaces()) {
		ns->ops->put(ns);
		return ERR_PTR(-EPERM);
	}
+2 −0
Original line number Diff line number Diff line
@@ -55,6 +55,8 @@ static __always_inline bool is_ns_init_id(const struct ns_common *ns)

#define ns_common_free(__ns) __ns_common_free(to_ns_common((__ns)))

bool may_see_all_namespaces(void);

static __always_inline __must_check int __ns_ref_active_read(const struct ns_common *ns)
{
	return atomic_read(&ns->__ns_ref_active);
+6 −0
Original line number Diff line number Diff line
@@ -309,3 +309,9 @@ void __ns_ref_active_get(struct ns_common *ns)
			return;
	}
}

bool may_see_all_namespaces(void)
{
	return (task_active_pid_ns(current) == &init_pid_ns) &&
	       ns_capable_noaudit(init_pid_ns.user_ns, CAP_SYS_ADMIN);
}
+4 −25
Original line number Diff line number Diff line
@@ -515,32 +515,11 @@ static inline bool __must_check ns_requested(const struct klistns *kls,
static inline bool __must_check may_list_ns(const struct klistns *kls,
					    struct ns_common *ns)
{
	if (kls->user_ns) {
		if (kls->userns_capable)
	if (kls->user_ns && kls->userns_capable)
		return true;
	} else {
		struct ns_common *owner;
		struct user_namespace *user_ns;

		owner = ns_owner(ns);
		if (owner)
			user_ns = to_user_ns(owner);
		else
			user_ns = &init_user_ns;
		if (ns_capable_noaudit(user_ns, CAP_SYS_ADMIN))
			return true;
	}

	if (is_current_namespace(ns))
		return true;

	if (ns->ns_type != CLONE_NEWUSER)
		return false;

	if (ns_capable_noaudit(to_user_ns(ns), CAP_SYS_ADMIN))
		return true;

	return false;
	return may_see_all_namespaces();
}

static inline void ns_put(struct ns_common *ns)
@@ -600,7 +579,7 @@ static ssize_t do_listns_userns(struct klistns *kls)

	ret = 0;
	head = &to_ns_common(kls->user_ns)->ns_owner_root.ns_list_head;
	kls->userns_capable = ns_capable_noaudit(kls->user_ns, CAP_SYS_ADMIN);
	kls->userns_capable = may_see_all_namespaces();

	rcu_read_lock();

+15 −10
Original line number Diff line number Diff line
@@ -37,17 +37,20 @@ FIXTURE(iterate_mount_namespaces) {
	__u64 mnt_ns_id[MNT_NS_COUNT];
};

static inline bool mntns_in_list(__u64 *mnt_ns_id, struct mnt_ns_info *info)
{
	for (int i = 0; i < MNT_NS_COUNT; i++) {
		if (mnt_ns_id[i] == info->mnt_ns_id)
			return true;
	}
	return false;
}

FIXTURE_SETUP(iterate_mount_namespaces)
{
	for (int i = 0; i < MNT_NS_COUNT; i++)
		self->fd_mnt_ns[i] = -EBADF;

	/*
	 * Creating a new user namespace let's us guarantee that we only see
	 * mount namespaces that we did actually create.
	 */
	ASSERT_EQ(unshare(CLONE_NEWUSER), 0);

	for (int i = 0; i < MNT_NS_COUNT; i++) {
		struct mnt_ns_info info = {};

@@ -75,13 +78,15 @@ TEST_F(iterate_mount_namespaces, iterate_all_forward)
	fd_mnt_ns_cur = fcntl(self->fd_mnt_ns[0], F_DUPFD_CLOEXEC);
	ASSERT_GE(fd_mnt_ns_cur, 0);

	for (;; count++) {
	for (;;) {
		struct mnt_ns_info info = {};
		int fd_mnt_ns_next;

		fd_mnt_ns_next = ioctl(fd_mnt_ns_cur, NS_MNT_GET_NEXT, &info);
		if (fd_mnt_ns_next < 0 && errno == ENOENT)
			break;
		if (mntns_in_list(self->mnt_ns_id, &info))
			count++;
		ASSERT_GE(fd_mnt_ns_next, 0);
		ASSERT_EQ(close(fd_mnt_ns_cur), 0);
		fd_mnt_ns_cur = fd_mnt_ns_next;
@@ -96,13 +101,15 @@ TEST_F(iterate_mount_namespaces, iterate_all_backwards)
	fd_mnt_ns_cur = fcntl(self->fd_mnt_ns[MNT_NS_LAST_INDEX], F_DUPFD_CLOEXEC);
	ASSERT_GE(fd_mnt_ns_cur, 0);

	for (;; count++) {
	for (;;) {
		struct mnt_ns_info info = {};
		int fd_mnt_ns_prev;

		fd_mnt_ns_prev = ioctl(fd_mnt_ns_cur, NS_MNT_GET_PREV, &info);
		if (fd_mnt_ns_prev < 0 && errno == ENOENT)
			break;
		if (mntns_in_list(self->mnt_ns_id, &info))
			count++;
		ASSERT_GE(fd_mnt_ns_prev, 0);
		ASSERT_EQ(close(fd_mnt_ns_cur), 0);
		fd_mnt_ns_cur = fd_mnt_ns_prev;
@@ -125,7 +132,6 @@ TEST_F(iterate_mount_namespaces, iterate_forward)
		ASSERT_GE(fd_mnt_ns_next, 0);
		ASSERT_EQ(close(fd_mnt_ns_cur), 0);
		fd_mnt_ns_cur = fd_mnt_ns_next;
		ASSERT_EQ(info.mnt_ns_id, self->mnt_ns_id[i]);
	}
}

@@ -144,7 +150,6 @@ TEST_F(iterate_mount_namespaces, iterate_backward)
		ASSERT_GE(fd_mnt_ns_prev, 0);
		ASSERT_EQ(close(fd_mnt_ns_cur), 0);
		fd_mnt_ns_cur = fd_mnt_ns_prev;
		ASSERT_EQ(info.mnt_ns_id, self->mnt_ns_id[i]);
	}
}