[Devel] [PATCH rh7] fs: drop useless d_root_check()

[Andrew Vagin]

On Fri, Dec 04, 2015 at 05:42:52PM +0300, Andrey Ryabinin wrote:
> d_root_check() takes the following locks in wrong order:
> 
> 	write_seqlock(&rename_lock);
> 	br_read_lock(&vfsmount_lock);
> 
> thus we may endup with deadlock.
> While we could fix it by taking these locks in reverse
> order, it would be better to just delete this function.
> 
> d_root_check() is a leftover from pre-mount namespace days,
> we don't need it today.
> 
> https://jira.sw.ru/browse/PSBM-41919
> 

Acked-by: Andrew Vagin <avagin at virtuozzo.com>

> Signed-off-by: Andrey Ryabinin <aryabinin at virtuozzo.com>
> ---
>  fs/dcache.c            | 53 --------------------------------------------------
>  fs/proc/base.c         | 31 +++++++++--------------------
>  fs/proc/fd.c           |  2 +-
>  include/linux/dcache.h |  1 -
>  4 files changed, 10 insertions(+), 77 deletions(-)
> 
> diff --git a/fs/dcache.c b/fs/dcache.c
> index a4f60d1..b5119e3 100644
> --- a/fs/dcache.c
> +++ b/fs/dcache.c
> @@ -3220,59 +3220,6 @@ Elong:
>  	return ERR_PTR(-ENAMETOOLONG);
>  }
>  
> -/**
> - * d_root_check - checks if dentry is accessible from current's fs root
> - * @dentry: dentry to be verified
> - * @vfsmnt: vfsmnt to which the dentry belongs
> - */
> -int d_root_check(struct path *path)
> -{
> -	struct dentry *dentry = path->dentry;
> -	struct vfsmount *vfsmnt = path->mnt;
> -	struct mount *mnt = real_mount(vfsmnt);
> -	struct path root;
> -	int error = 0;
> -
> -	if (ve_is_super(get_exec_env()))
> -		return 0;
> -
> -	if (path->dentry->d_op && path->dentry->d_op->d_dname)
> -		return 0;
> -
> -	/*
> -	 * If we meet disconnected entry, it's allowed
> -	 * to proceed, it comes from special allocations
> -	 * such as opening by file handle, where dentry
> -	 * formed directly from the inode opened.
> -	 */
> -	if (unlikely(dentry->d_flags & DCACHE_DISCONNECTED))
> -		return 0;
> -
> -	get_fs_root(current->fs, &root);
> -	write_seqlock(&rename_lock);
> -	br_read_lock(&vfsmount_lock);
> -	while (dentry != root.dentry || vfsmnt != root.mnt) {
> -		if (dentry == vfsmnt->mnt_root || IS_ROOT(dentry)) {
> -			/* Global root? */
> -			if (!mnt_has_parent(mnt)) {
> -				if (mnt->mnt_ns == ve0.ve_ns->mnt_ns)
> -					error = -EACCES;
> -				break;
> -			}
> -			dentry = mnt->mnt_mountpoint;
> -			mnt = mnt->mnt_parent;
> -			vfsmnt = &mnt->mnt;
> -			continue;
> -		}
> -		dentry = dentry->d_parent;
> -	}
> -	br_read_unlock(&vfsmount_lock);
> -	write_sequnlock(&rename_lock);
> -	path_put(&root);
> -
> -	return error;
> -}
> -
>  /*
>   * NOTE! The user-level library version returns a
>   * character pointer. The kernel system call just
> diff --git a/fs/proc/base.c b/fs/proc/base.c
> index b597b01..e9044e6 100644
> --- a/fs/proc/base.c
> +++ b/fs/proc/base.c
> @@ -165,13 +165,9 @@ static int get_task_root(struct task_struct *task, struct path *root)
>  	task_lock(task);
>  	if (task->fs) {
>  		get_fs_root(task->fs, root);
> -		task_unlock(task);
> -
> -		result = d_root_check(root);
> -		if (result)
> -			path_put(root);
> -	} else
> -		task_unlock(task);
> +		result = 0;
> +	}
> +	task_unlock(task);
>  	return result;
>  }
>  
> @@ -184,13 +180,9 @@ static int proc_cwd_link(struct dentry *dentry, struct path *path)
>  		task_lock(task);
>  		if (task->fs) {
>  			get_fs_pwd(task->fs, path);
> -			task_unlock(task);
> -
> -			result = d_root_check(path);
> -			if (result)
> -				path_put(path);
> -		} else
> -			task_unlock(task);
> +			result = 0;
> +		}
> +		task_unlock(task);
>  		put_task_struct(task);
>  	}
>  	return result;
> @@ -1491,15 +1483,10 @@ static int proc_exe_link(struct dentry *dentry, struct path *exe_path)
>  	exe_file = get_mm_exe_file(mm);
>  	mmput(mm);
>  	if (exe_file) {
> -		int result;
> -
> -		result = d_root_check(&exe_file->f_path);
> -		if (result == 0) {
> -			*exe_path = exe_file->f_path;
> -			path_get(&exe_file->f_path);
> -		}
> +		*exe_path = exe_file->f_path;
> +		path_get(&exe_file->f_path);
>  		fput(exe_file);
> -		return result;
> +		return 0;
>  	} else
>  		return -ENOENT;
>  }
> diff --git a/fs/proc/fd.c b/fs/proc/fd.c
> index cf657f7..911dac9 100644
> --- a/fs/proc/fd.c
> +++ b/fs/proc/fd.c
> @@ -158,7 +158,7 @@ static int proc_fd_link(struct dentry *dentry, struct path *path)
>  		spin_lock(&files->file_lock);
>  		fd_file = fcheck_files(files, fd);
>  		ret = -EACCES;
> -		if (fd_file && !d_root_check(&fd_file->f_path)) {
> +		if (fd_file) {
>  			*path = fd_file->f_path;
>  			path_get(&fd_file->f_path);
>  			ret = 0;
> diff --git a/include/linux/dcache.h b/include/linux/dcache.h
> index 56766a1..4a44f37 100644
> --- a/include/linux/dcache.h
> +++ b/include/linux/dcache.h
> @@ -333,7 +333,6 @@ extern char *d_absolute_path(const struct path *, char *, int);
>  extern char *d_path(const struct path *, char *, int);
>  extern char *dentry_path_raw(struct dentry *, char *, int);
>  extern char *dentry_path(struct dentry *, char *, int);
> -extern int d_root_check(struct path *path);
>  
>  /* Allocation counts.. */
>  
> -- 
> 2.4.10
>