Re: DoS with unprivileged mounts

[ebiederm@xmission.com (Eric W. Biederman)]

Miklos Szeredi <miklos@szeredi.hu> writes:

> On Wed, Aug 14, 2013 at 9:32 PM, Eric W. Biederman
> <ebiederm@xmission.com> wrote:
>
>>> The solution is also theoretically simple: mounts in unpriv namespaces
>>> are marked "volatile" and are dissolved on an unlink type operation.
>>>
>>> Such volatile mounts would be useful in general too.
>>
>> Agreed.
>>
>> This is a problem that is a general pain with mount namespaces in
>> general.
>>
>> I think the real technical hurdle is finding the mounts t in some random
>> mount namespace.  Once we can do that relatively efficiently the rest
>> becomes simple.
>
> We already have a "struct mountpoint" hashed on the dentry.  Chaining
> mounts on that mountpoint would be trivial.  And we need a
> MNT_VOLATILE flag and that's it.  If we fear that traversing the list
> of mounts on the dentry to check for non-volatile ones then we could
> also add a separate volatile counter to struct mountpoint and a
> matching flag to the dentry.  But I don't think that's really
> necessary.

*Blink* I had overlooked "struct mountpoint".  That indeed makes things
easier.

I agree we can chain "struct mount" on "struct mountpoint" and then we
would have an efficient implementation, that does not impact the vfs
fast path.

After that it becomes a question of permissions and semantics.

I am in the process of adopting the rule that something that is not
visible at the time we copy a set of mounts should not become visible
in the child mount namespace.  Grr.  This has been a busy month and
despite having been reviewed I haven't gotten around to pushing that
patch to linux-next.  

But MNT_VOLATILE by definition can not reveal anything becasue the
underlying mount point is removed, so that all of that weirdness is in
propogating mounts between mount namespaces is not relevant here.

This is however the propogation of an unmount between mount namespaces.

In general we don't even need the MNT_VOLATILE flag we just need the
appropriate permission checks.  However we do need something like
MNT_VOLATILE to prevent surprises.  MNT_VOLATILE would be used to
prevent things like:

# mount --bind /  /mnt
# rmdir /mnt/usr

I think the root user would be rather annoyed if that worked, so it does
appear we need something like MNT_VOLATILE.

Part of me does prefer the semantics Andy has suggested where instead of
unmounting things we have something like a skeleton of the mount tree
unioned with dcaches of the filesystems themselves.  With "struct
mountpoint" we are amazing close to that already.

A mount skeleton would allow us to always remove and rename directories
and files without really caring, about what mounts were present.
Probably with just a quick lookup to see if we need to set
DCACHE_MOUNTED.

The big practical problem I can see with MNT_VOLATILE is mount points in
shared directories like /tmp but without the sticky set.  At which point
it would be possible to delete another users mount points.  Perhaps we
need restrictions on where a user can mount.

Are there any practical limitations we can add to mount that will
ensure MNT_VOLATILE won't impact other users?  inode_capable looks like
a good canidate.  Which effectively boils down to /proc/<pid>/uid_map.

I will have to see if that semantic is workable, but at first glance it
looks promising.

So in summary I am thinking.

- Restrict on which files/directories an unprivileged user can mount.
  That immediately prevents most weirdness.

- Allow unlink and rmdir and rename to cause recursive lazy unmounts if
  the caller has the appropriate permissions.

  Perhaps the permissions needed include MNT_VOLATILE.

Any other suggestions?

Eric