linux-kernel - Re: [PATCH 23/34] union-mount: Prevent topmost file system from being mounted elsewhere

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20100930214716.GC490@shell>
Date:	Thu, 30 Sep 2010 17:47:16 -0400
From:	Valerie Aurora <vaurora@...hat.com>
To:	Miklos Szeredi <miklos@...redi.hu>
Cc:	viro@...iv.linux.org.uk, hch@...radead.org, agruen@...e.de,
	npiggin@...nel.dk, linux-kernel@...r.kernel.org,
	linux-fsdevel@...r.kernel.org
Subject: Re: [PATCH 23/34] union-mount: Prevent topmost file system from being mounted elsewhere

On Thu, Sep 30, 2010 at 11:37:48AM +0200, Miklos Szeredi wrote:
> On Thu, 16 Sep 2010, Valerie Aurora wrote:
> > The device underlying the topmost read-write layer of a file system
> > cannot be mounted anywhere else on the system.  We keep a pointer to
> > the union stack in the dentry of the topmost directory, so that dentry
> > can't be part of a different mount, since dentries are shared between
> > different mounts of the same device.
> > 
> > Signed-off-by: Valerie Aurora <vaurora@...hat.com>
> > ---
> >  fs/namespace.c |    5 +++++
> >  1 files changed, 5 insertions(+), 0 deletions(-)
> > 
> > diff --git a/fs/namespace.c b/fs/namespace.c
> > index 61256e6..26efaf3 100644
> > --- a/fs/namespace.c
> > +++ b/fs/namespace.c
> > @@ -1998,6 +1998,11 @@ int do_add_mount(struct vfsmount *newmnt, struct path *path,
> >  	if (S_ISLNK(newmnt->mnt_root->d_inode->i_mode))
> >  		goto unlock;
> >  
> > +	/* Top layers of union mounts can't be mounted elsewhere */
> > +	err = -EBUSY;
> > +	if (newmnt->mnt_sb->s_union_lower_mnts)
> > +		goto unlock;
> > +
> 
> This is insufficient: the super block may be mounted elsewhere later.
> And no, preventing bind mounts is not enough.

My mistake, that's a bug in the comment/commit message - s/mount/union
mount/.  The patch that prevents not-union mounts is:

    union-mount: Create check_topmost_union_mnt()
    
    check_topmost_union_mnt() checks that the topmost layer of a proposed
    union mount is read-write, supports fallthrus and whiteouts, and isn't
    mounted elsewhere.

And the patch that prevents bind mounts is:

    union-mount: Prevent bind mounts of union mounts
    
    Prevent bind mounts of parts of union mounts.
    
    XXX - Bind mounting parts of union mounts is probably easy to
    implement, but requires some careful thought about corner cases,
    extensive testing, and some refactoring of the code.

If you see any problems in those patches, I'd appreciate the comment.

> BTW, what about CLONE_NEWNS?  I think it's a rather big limitation if
> that doesn't work...

Great segue - I think the same code will make both CLONE_NEWNS and
bind mounts work.  We can allow multiple mounts of a union if it's the
exact same stack in each mount.  I will work on this.

-VAL
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/