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@xxxxxxxxxx> 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@xxxxxxxxxx> 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-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html