Re: [PATCH] [RFC] mnt: add ability to clone mntns starting with the current root

On Tue, Oct 7, 2014 at 5:20 PM, Eric W. Biederman [off-list ref] wrote:

Andy Lutomirski [off-list ref] writes:

quoted

On Tue, Oct 7, 2014 at 4:42 PM, Eric W. Biederman [off-list ref] wrote:

quoted

Andy Lutomirski [off-list ref] writes:

quoted

On Tue, Oct 7, 2014 at 3:42 PM, Eric W. Biederman [off-list ref] wrote:

quoted

Andy Lutomirski [off-list ref] writes:

quoted

On Tue, Oct 7, 2014 at 2:42 PM, Eric W. Biederman [off-list ref] wrote:

quoted

I am squinting and looking this way and that but while I can imagine
someone more clever than I can think up some unique property of rootfs
that makes it a little more exploitable than just mounting a ramfs,
but since you have to be root to exploit those properties I think the
game is pretty much lost.

Yes.  rootfs might not be empty, it might have totally insane
permissions, and it's globally shared, which makes it into a wonderful
channel to pass things around that shouldn't be passed around.

But if only root with proc mounted can reach it...  I don't know.

It doesn't have to be global root.  It could be userns root.

quoted

There might be a case for setting MNT_LOCKED when we overmount "/"
as root but I don't yet see it.

quoted

Can non-root do this?  You'd need to be in a userns with a "/" that
isn't MNT_LOCKED.  Can this happen on any normal setup?

FWIW, I think we should unconditionally MNT_LOCKED the root on userns
unshare, even if it's the only mount.

To the best of my knowledge MNT_LOCKED is set uncondintially on userns
unshare.

Only if list_empty(&old->mnt_expire), whatever that means, I think.

An autofs or nfs automounted mount.  Can those ever become root?

Dunno.

I thought that this code was what set MNT_LOCKED for things that
should be locked:

It does.

quoted

    /* Don't allow unprivileged users to reveal what is under a mount */
    if ((flag & CL_UNPRIVILEGED) && list_empty(&old->mnt_expire))
        mnt->mnt.mnt_flags |= MNT_LOCKED;

Now I'm confused.  Shouldn't that be checking for submounts?  Is that
what it's doing?

As it copies each mount (mostly submounts) it sets MNT_LOCKED.

Oh.  They're *all* MNT_LOCKED.  Duh.

quoted

Anyway, I think that this should unconditionally set MNT_LOCKED on the
thing that mounted on rootfs.

As I read the code mnt_set_expiry is only for nfs, cifs, and afs
submounts (and perhaps proc should use them). So as they are generated
mnt_expiry should never start on the root of filesystem of the mount tree.

Furthermore mnt_expiry is cleared when a mount is moved, and when
it is bind mounted, or propagated.

pivot_root does look as though it may be missing the proper mnt_expiry
handling list_del_init(&old->mnt_expire), but pivot_root does have
proper MNT_LOCKED handling.

pivot_root is quite broken, as noted in my other email.  It's just not
broken like this, I think. :)

Looking that test was slightly off and it should be:
        if ((flag & CL_UNPRIVILEGED) &&
            (!(flag & CL_EXPIRE) || list_empty(&old->mnt_expire))
                mnt->mnt.mnt_flags |= MNT_LOCKED;

So on that note we might clear CL_EXPIRE when CL_UNPRIVILEGED is set
in copy_tree but I don't see that as being really needed.

Eric

-- 
Andy Lutomirski
AMA Capital Management, LLC