linux-kernel - [PATCH] VFS: Sanity check mount flags passed to change_mnt

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>] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 26 Aug 2010 16:03:18 -0400
From:	Valerie Aurora <vaurora@...hat.com>
To:	Alexander Viro <viro@...iv.linux.org.uk>
Cc:	Karel Zak <kzak@...hat.com>, linux-fsdevel@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: [PATCH] VFS: Sanity check mount flags passed to change_mnt_propagation()

do_change_type() is buggy when passed multiple MS_* flags.  Discovered
because mount(8) incorrectly adds MS_RDONLY flag to
shared/slave/private/unbindable mounts.  Karel Zak will fix the
mount(8) bug shortly.

A test program is attached.  Against Viro's #untested branch.

-VAL

commit 208ca52f69ea53cf0723b8492fe54ebf9a3bf36a
Author: Valerie Aurora <vaurora@...hat.com>
Date:   Thu Aug 26 11:07:22 2010 -0700

    VFS: Sanity check mount flags passed to change_mnt_propagation()
    
    Sanity check the flags passed to change_mnt_propagation().  Exactly
    one flag should be set.  Return EINVAL otherwise.
    
    Userspace can pass in arbitrary combinations of MS_* flags to mount().
    do_change_type() is called if any of MS_SHARED, MS_PRIVATE, MS_SLAVE,
    or MS_UNBINDABLE is set.  do_change_type() clears MS_REC and then
    calls change_mnt_propagation() with the rest of the user-supplied
    flags.  change_mnt_propagation() clearly assumes only one flag is set
    but do_change_type() does not check that this is true.  For example,
    mount() with flags MS_SHARED | MS_RDONLY does not actually make the
    mount shared or read-only but does clear MNT_UNBINDABLE.
    
    Signed-off-by: Valerie Aurora <vaurora@...hat.com>

diff --git a/fs/namespace.c b/fs/namespace.c
index de402eb..4987c4c 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -1484,13 +1484,32 @@ out_unlock:
 }
 
 /*
+ * Sanity check the flags to change_mnt_propagation.
+ */
+
+static int flags_to_propagation_type(int flags) {
+	int type = flags & ~MS_REC;
+
+	/* Fail if any non-propagation flags are set */
+	if (type & ~(MS_SHARED | MS_PRIVATE | MS_SLAVE | MS_UNBINDABLE))
+		return 0;
+	/* Only one propagation flag should be set */
+	if (((type & (MS_SHARED)) && (type & ~MS_SHARED)) ||
+	    ((type & (MS_PRIVATE)) && (type & ~MS_PRIVATE)) ||
+	    ((type & (MS_SLAVE)) && (type & ~MS_SLAVE)) ||
+	    ((type & (MS_UNBINDABLE)) && (type & ~MS_UNBINDABLE)))
+		return 0;
+	return type;
+}
+
+/*
  * recursively change the type of the mountpoint.
  */
 static int do_change_type(struct path *path, int flag)
 {
 	struct vfsmount *m, *mnt = path->mnt;
 	int recurse = flag & MS_REC;
-	int type = flag & ~MS_REC;
+	int type;
 	int err = 0;
 
 	if (!capable(CAP_SYS_ADMIN))
@@ -1499,6 +1518,10 @@ static int do_change_type(struct path *path, int flag)
 	if (path->dentry != path->mnt->mnt_root)
 		return -EINVAL;
 
+	type = flags_to_propagation_type(flag);
+	if (!type)
+		return -EINVAL;
+
 	down_write(&namespace_sem);
 	if (type == MS_SHARED) {
 		err = invent_group_ids(mnt, recurse);

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/