On Sat, Aug 28, 2010 at 02:23:51PM -0700, Linus Torvalds wrote:
> On Thu, Aug 26, 2010 at 6:14 PM, Matthew Wilcox <matthew@xxxxxx> wrote:
> >
> > ? ? ? ?/* Only one propagation flag should be set, and no others */
> > ? ? ? ?if (hweight32(type) != 1
>
> Guys, stop with "teh crazy".
>
> What the f*ck kind of expression is that? We don't do this kind of
> crap. Even if it's possible that the compiler might be able to
> optimize it, it's just crazy to call "hweight32()" for something like
> this.
>
> Please think about what you're really after for a second, and realize
> that "one bit set" is just another way of saying "power of two". And
> then sit back, relax, and realize that there are way better ways to
> say "is this is a power of two" than counting bits, for chrissake!
I considered is_power_of_2() initially but rejected it as not
particularly readable. But hey, this is VFS code!
-VAL
commit ce3708ab514d850b0d0939f3fa6c64daad306a15
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..ddc5565 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -1484,13 +1484,29 @@ 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 (!is_power_of_2(type))
+ 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 +1515,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
