On Thu, 4 Sep 2008, Serge E. Hallyn wrote: > Quoting Miklos Szeredi (miklos@xxxxxxxxxx): > > On Thu, 4 Sep 2008, Serge E. Hallyn wrote: > > > but you're still doing > > > > > > if (IS_MNT_SHARED(old_nd.path.mnt) && !capable(CAP_SYS_ADMIN)) > > > goto out; > > > > > > shouldn't it be something like > > > > > > if (IS_MNT_SHARED(old_nd.path.mnt) && (old_nd.path.mnt & MNT_USER)) > > > goto out; > > > > > > ? > > > > Why would that be an error? There's no real security gain to be had > > from restricting a privileged user, but could cause a lot of > > annoyance. If we think this is dangerous, then protection should be > > built into mount(8) with an option to override. But not into the > > kernel, IMO. > > We disagree on that. But can we agree that the check you added is wrong? No :) > There is no reason why a user mount should not be able to do shared > mounts, is there? I don't know. It's something to think about in the future, but not essential. We know that without the above check the user can do bad things: propagate mounts back into the source, and we don't want that. We could allow binding a shared mount if a) the owners of the source and destination match b) the destination is made a slave of the source But the current patchset doesn't allow _any_ changes to propagation without CAP_SYS_ADMIN, so why should bind be an exception? And yes, this is something to think about, but I think it's a rather uncommon corner case, and so the patchset very much makes sense without having to deal with unprivileged mount propagation changes. > So should the check above just go away then? No, we'd be back with the original problem. Thanks, Miklos -- 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
