linux-kernel - Re: [PATCH 05/11] vfs: Add a function to lazily unmount all mounts from any dentry.

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]

Date:	Sat, 15 Feb 2014 17:50:33 -0800
From:	ebiederm@...ssion.com (Eric W. Biederman)
To:	Al Viro <viro@...iv.linux.org.uk>
Cc:	"Serge E. Hallyn" <serge@...lyn.com>,
	Linux-Fsdevel <linux-fsdevel@...r.kernel.org>,
	Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Andy Lutomirski <luto@...capital.net>,
	Rob Landley <rob@...dley.net>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Miklos Szeredi <miklos@...redi.hu>,
	Christoph Hellwig <hch@...radead.org>,
	Karel Zak <kzak@...hat.com>,
	"J. Bruce Fields" <bfields@...ldses.org>
Subject: Re: [PATCH 05/11] vfs: Add a function to lazily unmount all mounts from any dentry.

ebiederm@...ssion.com (Eric W. Biederman) writes:

> v2: Always drop the lock when exiting early.
> v3: Make detach_mounts robust about freeing several
>     mounts on the same mountpoint at one time, and remove
>     the unneeded mnt_list list test.
> v4: Document the purpose of detach_mounts and why new_mountpoint is
>     safe to call.
>
> Signed-off-by: Eric W. Biederman <ebiederman@...tter.com>
> ---
>  fs/mount.h     |    2 ++
>  fs/namespace.c |   39 +++++++++++++++++++++++++++++++++++++++
>  2 files changed, 41 insertions(+), 0 deletions(-)
>
> diff --git a/fs/mount.h b/fs/mount.h
> index 50a72d46e7a6..2b470f34e665 100644
> --- a/fs/mount.h
> +++ b/fs/mount.h
> @@ -84,6 +84,8 @@ extern struct mount *__lookup_mnt_last(struct vfsmount *, struct dentry *);
>  
>  extern bool legitimize_mnt(struct vfsmount *, unsigned);
>  
> +extern void detach_mounts(struct dentry *dentry);

After Linus's rant about performance I took a second look, and lo and
behold there is a significant optimization oppportunity missed here.

This should almost certainly be:

extern void __detach_mounts(struct dentry *dentry);

static inline void detach_mounts(struct dentry *dentry)
{
	if (unlikely(d_mountpoint(dentry)))
        	return;
	__detach_mounts(dentry);
}

Just so I don't take the namespace lock when not necessary.

Although wether this should be inline or at the start of detach_mounts
and out of line I don't know.  Sigh.  I will have to play with this
detail a bit more.

Eric



>  static inline void get_mnt_ns(struct mnt_namespace *ns)
>  {
>  	atomic_inc(&ns->count);
> diff --git a/fs/namespace.c b/fs/namespace.c
> index 33db9e95bd5c..7abbf722ce18 100644
> --- a/fs/namespace.c
> +++ b/fs/namespace.c
> @@ -1359,6 +1359,45 @@ static int do_umount(struct mount *mnt, int flags)
>  	return retval;
>  }
>  
> +/*
> + * detach_mounts - lazily unmount all mounts on the specified dentry
> + *
> + * During unlink, rmdir, and d_drop it is possible to loose the path
> + * to an existing mountpoint, and wind up leaking the mount.
> + * detach_mounts allows lazily unmounting those mounts instead of
> + * leaking them.
> + * 
> + * The caller may hold dentry->d_inode->i_mutex.
> + */
> +void detach_mounts(struct dentry *dentry)
> +{
> +	struct mountpoint *mp;
> +	struct mount *mnt;
> +
> +	namespace_lock();
> +	if (!d_mountpoint(dentry))
> +		goto out_unlock;
> +
> +	/* 
> +	 * The namespace lock and d_mountpoint being set guarantees
> +	 * that new_mountpoint will just be a lookup of the existing
> +	 * mountpoint structure.
> +	 */
> +	mp = new_mountpoint(dentry);
> +	if (IS_ERR(mp))
> +		goto out_unlock;
> +
> +	lock_mount_hash();
> +	while (!list_empty(&mp->m_list)) {
> +		mnt = list_first_entry(&mp->m_list, struct mount, mnt_mp_list);
> +		umount_tree(mnt, 1);
> +	}
> +	unlock_mount_hash();
> +	put_mountpoint(mp);
> +out_unlock:
> +	namespace_unlock();
> +}
> +
>  /* 
>   * Is the caller allowed to modify his namespace?
>   */
--
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/