[pve-devel] [PATCH pve-kernel 1/3] Add 3 Patches addressing security issues

Stoiko Ivanov s.ivanov at proxmox.com
Wed Nov 14 20:25:26 CET 2018


* CVE-2018-18955 (https://launchpad.net/bugs/1801924) is addressed by
  0009-userns-also-map-extents-in-the-reverse-map-to-kernel.patch
* https://launchpad.net/bugs/1789161 is addressed by the other 2 patches. (see
  the link for a reproducer)

Signed-off-by: Stoiko Ivanov <s.ivanov at proxmox.com>
---
 ...extents-in-the-reverse-map-to-kernel.patch | 75 ++++++++++++++++++
 ...mount-Retest-MNT_LOCKED-in-do_umount.patch | 67 ++++++++++++++++
 ...w-copying-MNT_UNBINDABLE-MNT_LOCKED-.patch | 78 +++++++++++++++++++
 3 files changed, 220 insertions(+)
 create mode 100644 patches/kernel/0009-userns-also-map-extents-in-the-reverse-map-to-kernel.patch
 create mode 100644 patches/kernel/0010-mount-Retest-MNT_LOCKED-in-do_umount.patch
 create mode 100644 patches/kernel/0011-mount-Don-t-allow-copying-MNT_UNBINDABLE-MNT_LOCKED-.patch

diff --git a/patches/kernel/0009-userns-also-map-extents-in-the-reverse-map-to-kernel.patch b/patches/kernel/0009-userns-also-map-extents-in-the-reverse-map-to-kernel.patch
new file mode 100644
index 0000000..2e88972
--- /dev/null
+++ b/patches/kernel/0009-userns-also-map-extents-in-the-reverse-map-to-kernel.patch
@@ -0,0 +1,75 @@
+From 5506202b83e65b844309093e712b5b507eb1e403 Mon Sep 17 00:00:00 2001
+From: Jann Horn <jannh at google.com>
+Date: Tue, 13 Nov 2018 07:42:38 +0000
+Subject: [PATCH 09/11] userns: also map extents in the reverse map to kernel
+ IDs
+
+BugLink: https://launchpad.net/bugs/1801924
+
+The current logic first clones the extent array and sorts both copies, then
+maps the lower IDs of the forward mapping into the lower namespace, but
+doesn't map the lower IDs of the reverse mapping.
+
+This means that code in a nested user namespace with >5 extents will see
+incorrect IDs. It also breaks some access checks, like
+inode_owner_or_capable() and privileged_wrt_inode_uidgid(), so a process
+can incorrectly appear to be capable relative to an inode.
+
+To fix it, we have to make sure that the "lower_first" members of extents
+in both arrays are translated; and we have to make sure that the reverse
+map is sorted *after* the translation (since otherwise the translation can
+break the sorting).
+
+This is CVE-2018-18955.
+
+Fixes: 6397fac4915a ("userns: bump idmap limits to 340")
+Cc: stable at vger.kernel.org
+Signed-off-by: Jann Horn <jannh at google.com>
+Tested-by: Eric W. Biederman <ebiederm at xmission.com>
+Reviewed-by: Eric W. Biederman <ebiederm at xmission.com>
+Signed-off-by: Eric W. Biederman <ebiederm at xmission.com>
+
+CVE-2018-18955
+
+(cherry picked from commit d2f007dbe7e4c9583eea6eb04d60001e85c6f1bd)
+Signed-off-by: Tyler Hicks <tyhicks at canonical.com>
+Acked-by: Colin King <colin.king at canonical.com>
+Acked-by: Thadeu Lima de Souza Cascardo <cascardo at canonical.com>
+Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo at canonical.com>
+---
+ kernel/user_namespace.c | 12 ++++++++----
+ 1 file changed, 8 insertions(+), 4 deletions(-)
+
+diff --git a/kernel/user_namespace.c b/kernel/user_namespace.c
+index 08d638386b83..12de8c144db9 100644
+--- a/kernel/user_namespace.c
++++ b/kernel/user_namespace.c
+@@ -983,10 +983,6 @@ static ssize_t map_write(struct file *file, const char __user *buf,
+ 	if (!new_idmap_permitted(file, ns, cap_setid, &new_map))
+ 		goto out;
+ 
+-	ret = sort_idmaps(&new_map);
+-	if (ret < 0)
+-		goto out;
+-
+ 	ret = -EPERM;
+ 	/* Map the lower ids from the parent user namespace to the
+ 	 * kernel global id space.
+@@ -1013,6 +1009,14 @@ static ssize_t map_write(struct file *file, const char __user *buf,
+ 		e->lower_first = lower_first;
+ 	}
+ 
++	/*
++	 * If we want to use binary search for lookup, this clones the extent
++	 * array and sorts both copies.
++	 */
++	ret = sort_idmaps(&new_map);
++	if (ret < 0)
++		goto out;
++
+ 	/* Install the map */
+ 	if (new_map.nr_extents <= UID_GID_MAP_MAX_BASE_EXTENTS) {
+ 		memcpy(map->extent, new_map.extent,
+-- 
+2.11.0
+
diff --git a/patches/kernel/0010-mount-Retest-MNT_LOCKED-in-do_umount.patch b/patches/kernel/0010-mount-Retest-MNT_LOCKED-in-do_umount.patch
new file mode 100644
index 0000000..4476eeb
--- /dev/null
+++ b/patches/kernel/0010-mount-Retest-MNT_LOCKED-in-do_umount.patch
@@ -0,0 +1,67 @@
+From 3918a0379c7d37ce5d348ec6c2439d744a92a1f8 Mon Sep 17 00:00:00 2001
+From: "Eric W. Biederman" <ebiederm at xmission.com>
+Date: Tue, 13 Nov 2018 07:44:37 +0000
+Subject: [PATCH 10/11] mount: Retest MNT_LOCKED in do_umount
+
+BugLink: https://launchpad.net/bugs/1789161
+
+It was recently pointed out that the one instance of testing MNT_LOCKED
+outside of the namespace_sem is in ksys_umount.
+
+Fix that by adding a test inside of do_umount with namespace_sem and
+the mount_lock held.  As it helps to fail fails the existing test is
+maintained with an additional comment pointing out that it may be racy
+because the locks are not held.
+
+Cc: stable at vger.kernel.org
+Reported-by: Al Viro <viro at ZenIV.linux.org.uk>
+Fixes: 5ff9d8a65ce8 ("vfs: Lock in place mounts from more privileged users")
+Signed-off-by: "Eric W. Biederman" <ebiederm at xmission.com>
+(cherry picked from commit 25d202ed820ee347edec0bf3bf553544556bf64b)
+Signed-off-by: Tyler Hicks <tyhicks at canonical.com>
+Acked-by: Colin King <colin.king at canonical.com>
+Acked-by: Thadeu Lima de Souza Cascardo <cascardo at canonical.com>
+Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo at canonical.com>
+---
+ fs/namespace.c | 10 ++++++++--
+ 1 file changed, 8 insertions(+), 2 deletions(-)
+
+diff --git a/fs/namespace.c b/fs/namespace.c
+index 570c9672ac9f..dcf107925150 100644
+--- a/fs/namespace.c
++++ b/fs/namespace.c
+@@ -1609,8 +1609,13 @@ static int do_umount(struct mount *mnt, int flags)
+ 
+ 	namespace_lock();
+ 	lock_mount_hash();
+-	event++;
+ 
++	/* Recheck MNT_LOCKED with the locks held */
++	retval = -EINVAL;
++	if (mnt->mnt.mnt_flags & MNT_LOCKED)
++		goto out;
++
++	event++;
+ 	if (flags & MNT_DETACH) {
+ 		if (!list_empty(&mnt->mnt_list))
+ 			umount_tree(mnt, UMOUNT_PROPAGATE);
+@@ -1624,6 +1629,7 @@ static int do_umount(struct mount *mnt, int flags)
+ 			retval = 0;
+ 		}
+ 	}
++out:
+ 	unlock_mount_hash();
+ 	namespace_unlock();
+ 	return retval;
+@@ -1714,7 +1720,7 @@ SYSCALL_DEFINE2(umount, char __user *, name, int, flags)
+ 		goto dput_and_out;
+ 	if (!check_mnt(mnt))
+ 		goto dput_and_out;
+-	if (mnt->mnt.mnt_flags & MNT_LOCKED)
++	if (mnt->mnt.mnt_flags & MNT_LOCKED) /* Check optimistically */
+ 		goto dput_and_out;
+ 	retval = -EPERM;
+ 	if (flags & MNT_FORCE && !capable(CAP_SYS_ADMIN))
+-- 
+2.11.0
+
diff --git a/patches/kernel/0011-mount-Don-t-allow-copying-MNT_UNBINDABLE-MNT_LOCKED-.patch b/patches/kernel/0011-mount-Don-t-allow-copying-MNT_UNBINDABLE-MNT_LOCKED-.patch
new file mode 100644
index 0000000..55562d9
--- /dev/null
+++ b/patches/kernel/0011-mount-Don-t-allow-copying-MNT_UNBINDABLE-MNT_LOCKED-.patch
@@ -0,0 +1,78 @@
+From 37b0e20be5149d5dc049e2aed3e8b03589a6ffa0 Mon Sep 17 00:00:00 2001
+From: "Eric W. Biederman" <ebiederm at xmission.com>
+Date: Tue, 13 Nov 2018 07:44:38 +0000
+Subject: [PATCH 11/11] mount: Don't allow copying MNT_UNBINDABLE|MNT_LOCKED
+ mounts
+
+BugLink: https://launchpad.net/bugs/1789161
+
+Jonathan Calmels from NVIDIA reported that he's able to bypass the
+mount visibility security check in place in the Linux kernel by using
+a combination of the unbindable property along with the private mount
+propagation option to allow a unprivileged user to see a path which
+was purposefully hidden by the root user.
+
+Reproducer:
+  # Hide a path to all users using a tmpfs
+  root at castiana:~# mount -t tmpfs tmpfs /sys/devices/
+  root at castiana:~#
+
+  # As an unprivileged user, unshare user namespace and mount namespace
+  stgraber at castiana:~$ unshare -U -m -r
+
+  # Confirm the path is still not accessible
+  root at castiana:~# ls /sys/devices/
+
+  # Make /sys recursively unbindable and private
+  root at castiana:~# mount --make-runbindable /sys
+  root at castiana:~# mount --make-private /sys
+
+  # Recursively bind-mount the rest of /sys over to /mnnt
+  root at castiana:~# mount --rbind /sys/ /mnt
+
+  # Access our hidden /sys/device as an unprivileged user
+  root at castiana:~# ls /mnt/devices/
+  breakpoint cpu cstate_core cstate_pkg i915 intel_pt isa kprobe
+  LNXSYSTM:00 msr pci0000:00 platform pnp0 power software system
+  tracepoint uncore_arb uncore_cbox_0 uncore_cbox_1 uprobe virtual
+
+Solve this by teaching copy_tree to fail if a mount turns out to be
+both unbindable and locked.
+
+Cc: stable at vger.kernel.org
+Fixes: 5ff9d8a65ce8 ("vfs: Lock in place mounts from more privileged users")
+Reported-by: Jonathan Calmels <jcalmels at nvidia.com>
+Signed-off-by: "Eric W. Biederman" <ebiederm at xmission.com>
+(cherry picked from commit df7342b240185d58d3d9665c0bbf0a0f5570ec29)
+Signed-off-by: Tyler Hicks <tyhicks at canonical.com>
+Acked-by: Colin King <colin.king at canonical.com>
+Acked-by: Thadeu Lima de Souza Cascardo <cascardo at canonical.com>
+Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo at canonical.com>
+---
+ fs/namespace.c | 10 ++++++++--
+ 1 file changed, 8 insertions(+), 2 deletions(-)
+
+diff --git a/fs/namespace.c b/fs/namespace.c
+index dcf107925150..91a3040f0cd0 100644
+--- a/fs/namespace.c
++++ b/fs/namespace.c
+@@ -1798,8 +1798,14 @@ struct mount *copy_tree(struct mount *mnt, struct dentry *dentry,
+ 		for (s = r; s; s = next_mnt(s, r)) {
+ 			if (!(flag & CL_COPY_UNBINDABLE) &&
+ 			    IS_MNT_UNBINDABLE(s)) {
+-				s = skip_mnt_tree(s);
+-				continue;
++				if (s->mnt.mnt_flags & MNT_LOCKED) {
++					/* Both unbindable and locked. */
++					q = ERR_PTR(-EPERM);
++					goto out;
++				} else {
++					s = skip_mnt_tree(s);
++					continue;
++				}
+ 			}
+ 			if (!(flag & CL_COPY_MNT_NS_FILE) &&
+ 			    is_mnt_ns_file(s->mnt.mnt_root)) {
+-- 
+2.11.0
+
-- 
2.19.1





More information about the pve-devel mailing list