Re: [PATCH] ovl: redirect_dir=nofollow should not follow redirect for opaque lower

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Feb 2, 2018 at 5:23 PM, Vivek Goyal <vgoyal@xxxxxxxxxx> wrote:
> redirect_dir=nofollow should not follow a redirect. But in a specific
> configuration it can still follow it.  For example try this.
>
> $ mkdir -p lower0 lower1/foo upper work merged
> $ touch lower1/foo/lower-file.txt
> $ setfattr -n "trusted.overlay.opaque" -v "y" lower1/foo
> $ mount -t overlay -o lowerdir=lower1:lower0,workdir=work,upperdir=upper,redirect_dir=on none merged
> $ cd merged
> $ mv foo foo-renamed
> $ umount merged
>
> # mount again. This time with redirect_dir=nofollow
> $ mount -t overlay -o lowerdir=lower1:lower0,workdir=work,upperdir=upper,redirect_dir=nofollow none merged
> $ ls merged/foo-renamed/
> # This lists lower-file.txt, while it should not have.
>
> Basically, we are doing redirect check after we check for d.stop. And
> if this is not last lower, and we find an opaque lower, d.stop will be
> set.
>
> ovl_lookup_single()
>         if (!d->last && ovl_is_opaquedir(this)) {
>                 d->stop = d->opaque = true;
>                 goto out;
>         }
>
> To fix this, first check redirect is allowed. And after that check if
> d.stop has been set or not.
>
> Signed-off-by: Vivek Goyal <vgoyal@xxxxxxxxxx>

Looks good.
Probably should have Fixes and Cc: stable

> ---
>  fs/overlayfs/namei.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/fs/overlayfs/namei.c b/fs/overlayfs/namei.c
> index beb945e1963c..ef3e7ea76296 100644
> --- a/fs/overlayfs/namei.c
> +++ b/fs/overlayfs/namei.c
> @@ -678,9 +678,6 @@ struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry,
>                 stack[ctr].layer = lower.layer;
>                 ctr++;
>
> -               if (d.stop)
> -                       break;
> -
>                 /*
>                  * Following redirects can have security consequences: it's like
>                  * a symlink into the lower layer without the permission checks.
> @@ -697,6 +694,9 @@ struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry,
>                         goto out_put;
>                 }
>
> +               if (d.stop)
> +                       break;
> +
>                 if (d.redirect && d.redirect[0] == '/' && poe != roe) {
>                         poe = roe;
>
> --
> 2.13.6
>
--
To unsubscribe from this list: send the line "unsubscribe linux-unionfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Filesystems Devel]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux