[Devel,RH7] ms/ovl: fix d_real() for stacked fs

Submitted by Pavel Tikhomirov on Feb. 9, 2017, 4:13 p.m.

Details

Message ID 20170209161348.10540-1-ptikhomirov@virtuozzo.com
State New
Series "ms/ovl: fix d_real() for stacked fs"
Headers show

Commit Message

Pavel Tikhomirov Feb. 9, 2017, 4:13 p.m.
From: Miklos Szeredi <mszeredi@redhat.com>

Handling of recursion in d_real() is completely broken.  Recursion is only
done in the 'inode != NULL' case.  But when opening the file we have
'inode == NULL' hence d_real() will return an overlay dentry.  This won't
work since overlayfs doesn't define its own file operations, so all file
ops will fail.

Fix by doing the recursion first and the check against the inode second.

Bash script to reproduce the issue written by Quentin:

 - 8< - - - - - 8< - - - - - 8< - - - - - 8< - - - -
tmpdir=$(mktemp -d)
pushd ${tmpdir}

mkdir -p {upper,lower,work}
echo -n 'rocks' > lower/ksplice
mount -t overlay level_zero upper -o lowerdir=lower,upperdir=upper,workdir=work
cat upper/ksplice

tmpdir2=$(mktemp -d)
pushd ${tmpdir2}

mkdir -p {upper,work}
mount -t overlay level_one upper -o lowerdir=${tmpdir}/upper,upperdir=upper,workdir=work
ls -l upper/ksplice
cat upper/ksplice
 - 8< - - - - - 8< - - - - - 8< - - - - - 8< - - - -

found while investigating https://jira.sw.ru/browse/PSBM-58480

Reported-by: Quentin Casasnovas <quentin.casasnovas@oracle.com>
Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
Fixes: 2d902671ce1c ("vfs: merge .d_select_inode() into .d_real()")
Cc: <stable@vger.kernel.org> # v4.8+
Signed-off-by: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>
---
 fs/overlayfs/super.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Patch hide | download patch | download mbox

diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
index edd46a0..0e10085 100644
--- a/fs/overlayfs/super.c
+++ b/fs/overlayfs/super.c
@@ -328,11 +328,11 @@  static struct dentry *ovl_d_real(struct dentry *dentry,
 	if (!real)
 		goto bug;
 
+	/* Handle recursion */
+	real = d_real(real, inode, open_flags);
+
 	if (!inode || inode == d_inode(real))
 		return real;
-
-	/* Handle recursion */
-	return d_real(real, inode, open_flags);
 bug:
 	WARN(1, "ovl_d_real(%pd4, %s:%lu): real dentry not found\n", dentry,
 	     inode ? inode->i_sb->s_id : "NULL", inode ? inode->i_ino : 0);

Comments

Pavel Tikhomirov Feb. 13, 2017, 1:22 p.m.
Maxim, please review.

On 02/09/2017 07:13 PM, Pavel Tikhomirov wrote:
> From: Miklos Szeredi <mszeredi@redhat.com>
>
> Handling of recursion in d_real() is completely broken.  Recursion is only
> done in the 'inode != NULL' case.  But when opening the file we have
> 'inode == NULL' hence d_real() will return an overlay dentry.  This won't
> work since overlayfs doesn't define its own file operations, so all file
> ops will fail.
>
> Fix by doing the recursion first and the check against the inode second.
>
> Bash script to reproduce the issue written by Quentin:
>
>  - 8< - - - - - 8< - - - - - 8< - - - - - 8< - - - -
> tmpdir=$(mktemp -d)
> pushd ${tmpdir}
>
> mkdir -p {upper,lower,work}
> echo -n 'rocks' > lower/ksplice
> mount -t overlay level_zero upper -o lowerdir=lower,upperdir=upper,workdir=work
> cat upper/ksplice
>
> tmpdir2=$(mktemp -d)
> pushd ${tmpdir2}
>
> mkdir -p {upper,work}
> mount -t overlay level_one upper -o lowerdir=${tmpdir}/upper,upperdir=upper,workdir=work
> ls -l upper/ksplice
> cat upper/ksplice
>  - 8< - - - - - 8< - - - - - 8< - - - - - 8< - - - -
>
> found while investigating https://jira.sw.ru/browse/PSBM-58480
>
> Reported-by: Quentin Casasnovas <quentin.casasnovas@oracle.com>
> Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
> Fixes: 2d902671ce1c ("vfs: merge .d_select_inode() into .d_real()")
> Cc: <stable@vger.kernel.org> # v4.8+
> Signed-off-by: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>
> ---
>  fs/overlayfs/super.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
> index edd46a0..0e10085 100644
> --- a/fs/overlayfs/super.c
> +++ b/fs/overlayfs/super.c
> @@ -328,11 +328,11 @@ static struct dentry *ovl_d_real(struct dentry *dentry,
>  	if (!real)
>  		goto bug;
>
> +	/* Handle recursion */
> +	real = d_real(real, inode, open_flags);
> +
>  	if (!inode || inode == d_inode(real))
>  		return real;
> -
> -	/* Handle recursion */
> -	return d_real(real, inode, open_flags);
>  bug:
>  	WARN(1, "ovl_d_real(%pd4, %s:%lu): real dentry not found\n", dentry,
>  	     inode ? inode->i_sb->s_id : "NULL", inode ? inode->i_ino : 0);
>