[Devel] [PATCH rh7] fs: hold reference on original path

[Konstantin Khorenko]

Kirill, please review.

--
Best regards,

Konstantin Khorenko,
Virtuozzo Linux Kernel Team

On 09/22/2016 03:15 AM, Maxim Patlasov wrote:
> struct file holds references on its f_path.mnt and f_path.dentry by calling
> path_get(&f->f_path) from do_dentry_open(). Let's use the same technique
> for f->f_original_path. Otherwise, f_original_path.dentry can be deleted while
> file still references it leading to NULL-ptr-deref on f->f_original_path.dentry->d_inode.
>
> https://jira.sw.ru/browse/PSBM-52373
>
> Signed-off-by: Maxim Patlasov <mpatlasov at virtuozzo.com>
> ---
>  fs/file_table.c |    6 ++++++
>  fs/open.c       |   18 +++++++++++++++---
>  2 files changed, 21 insertions(+), 3 deletions(-)
>
> diff --git a/fs/file_table.c b/fs/file_table.c
> index 957c476..b8982d8 100644
> --- a/fs/file_table.c
> +++ b/fs/file_table.c
> @@ -242,6 +242,8 @@ static void __fput(struct file *file)
>  	struct dentry *dentry = file->f_path.dentry;
>  	struct vfsmount *mnt = file->f_path.mnt;
>  	struct inode *inode = dentry->d_inode;
> +	struct dentry *original_dentry = file->f_original_path.dentry;
> +	struct vfsmount *original_mnt = file->f_original_path.mnt;
>
>  	might_sleep();
>
> @@ -273,10 +275,14 @@ static void __fput(struct file *file)
>  		drop_file_write_access(file);
>  	file->f_path.dentry = NULL;
>  	file->f_path.mnt = NULL;
> +	file->f_original_path.dentry = NULL;
> +	file->f_original_path.mnt = NULL;
>  	file->f_inode = NULL;
>  	file_free(file);
>  	dput(dentry);
>  	mntput(mnt);
> +	dput(original_dentry);
> +	mntput(original_mnt);
>  }
>
>  static DEFINE_SPINLOCK(delayed_fput_lock);
> diff --git a/fs/open.c b/fs/open.c
> index 8c066b1..25dbc85 100644
> --- a/fs/open.c
> +++ b/fs/open.c
> @@ -893,16 +893,28 @@ int vfs_open(const struct path *path, struct file *filp,
>  {
>  	struct inode *inode = path->dentry->d_inode;
>  	iop_dentry_open_t dentry_open = get_dentry_open_iop(inode);
> +	int do_cleanup = 0;
> +	int ret;
>
> -	if (!filp->f_original_path.mnt)
> +	if (!filp->f_original_path.mnt) {
>  		filp->f_original_path = *path;
> +		path_get(&filp->f_original_path);
> +		do_cleanup = 1;
> +	}
>
>  	if (dentry_open)
> -		return dentry_open(path->dentry, filp, cred);
> +		ret = dentry_open(path->dentry, filp, cred);
>  	else {
>  		filp->f_path = *path;
> -		return do_dentry_open(filp, NULL, cred);
> +		ret = do_dentry_open(filp, NULL, cred);
>  	}
> +
> +	if (ret && do_cleanup) {
> +		path_put(&filp->f_original_path);
> +		filp->f_original_path.mnt = NULL;
> +		filp->f_original_path.dentry = NULL;
> +	}
> +	return ret;
>  }
>  EXPORT_SYMBOL(vfs_open);
>
>
> .
>