On Mon, Mar 12, 2018 at 2:51 PM, Vivek Goyal <vgoyal@xxxxxxxxxx> wrote:
> On Sat, Mar 10, 2018 at 10:07:07PM +0200, Amir Goldstein wrote:
>> On Fri, Mar 9, 2018 at 10:44 PM, Vivek Goyal <vgoyal@xxxxxxxxxx> wrote:
>> > From: Amir Goldstein <amir73il@xxxxxxxxx>
>> >
>> > As of now if we encounter an opaque dir while looking for a dentry, we set
>> > d->last=true. This means that there is no need to look further in any of
>> > the lower layers. This works fine as long as there are no redirets or
>> > relative redircts. But what if there is an absolute redirect on the
>> > children dentry of opaque directory. We still need to continue to look into
>> > next lower layer. This patch fixes it.
>> >
>> > Here is first example to demonstrate the issue. Say you have following setup.
>> >
>> > upper: /redirect (redirect=/a/b/c)
>> > lower1: /a/[b]/c ([b] is opaque) (c has absolute redirect=/a/b/d/foo)
>>
>> According to the text below you meant redirect=/a/b/d
>
> Will fix.
>
>>
>> > lower0: /a/b/d/foo
>> >
>> > Now "redirect" dir should merge with lower1:/a/b/c/ and lower0:/a/b/d. Note,
>> > despite the fact lower1:/a/[b] is opaque, we need to continue to look into
>> > lower0 because children c has an absolute redirect.
>> >
>> > Following is second example.
>>
>> Let's refer to the below as a reproducer then
>
> Ok.
>
>>
>> >
>> > Watch me make foo disappear:
>> >
>> > $ mkdir lower middle upper work work2 merged
>> > $ mkdir lower/origin
>> > $ touch lower/origin/foo
>> > $ mount -t overlay none merged/ \
>> > -olowerdir=lower,upperdir=middle,workdir=work2
>> > $ mkdir merged/pure
>> > $ mv merged/origin merged/pure/redirect
>> > $ umount merged
>> > $ mount -t overlay none merged/ \
>> > -olowerdir=middle:lower,upperdir=upper,workdir=work
>> > $ mv merged/pure/redirect merged/redirect
>> >
>> > Now you see foo inside a twice redirected merged dir:
>> >
>> > $ ls merged/redirect
>> > foo
>> > $ umount merged
>> > $ mount -t overlay none merged/ \
>> > -olowerdir=middle:lower,upperdir=upper,workdir=work
>> >
>> > After mount cycle you don't see foo inside the same dir:
>> >
>> > $ ls merged/redirect
>> >
>> > During middle layer lookup, the opaqueness of middle/pure is left in
>> > the lookup state and then middle/pure/redirect is wrongly treated as
>> > opaque.
>> >
>> > Fixes: 02b69b284cd7 ("ovl: lookup redirects")
>> > Cc: <stable@xxxxxxxxxxxxxxx> #v4.10
>> > Signed-off-by: Amir Goldstein <amir73il@xxxxxxxxx>
>> > Signed-off-by: Vivek Goyal <vgoyal@xxxxxxxxxx>
>> > ---
>> > fs/overlayfs/namei.c | 8 ++++++++
>> > 1 file changed, 8 insertions(+)
>> >
>> > diff --git a/fs/overlayfs/namei.c b/fs/overlayfs/namei.c
>> > index de08a67405d0..096d5853af7d 100644
>> > --- a/fs/overlayfs/namei.c
>> > +++ b/fs/overlayfs/namei.c
>> > @@ -56,6 +56,14 @@ static int ovl_check_redirect(struct dentry *dentry, struct ovl_lookup_data *d,
>> > if (s == next)
>> > goto invalid;
>> > }
>> > + /*
>> > + * One of the ancestor path elements in an absolute path
>> > + * lookup in ovl_lookup_layer() could have been opaque, but it
>> > + * possible for a decendant path element to reset opaqueness in
>> > + * case this path element has an absolute redirect to a lower
>> > + * layer.
>> > + */
>>
>> Now the comment is out of sync with new semantics, it is not opaquness
>> that is being reset. it's stop and continue lookup in lower layers.
>
> Will change. How about following.
>
> One of the ancestor path elements in an absolute path
> lookup in ovl_lookup_layer() could have been opaque and that will
> stop further lookup in lower layers (d->stop = true). But we have
> found an absolute redirect in decendant path element and that should
> force continue lookup in lower layers (reset d->stop).
>
Good.
Thanks,
Amir.
--
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
