Re: [RFC][PATCH] overlay: always follow metacopy into data-only layers

[Giuseppe Scrivano <giuseppe@scrivano.org>]

Hi Amir,

Amir Goldstein <amir73il@gmail.com> writes:

> On Thu, Nov 5, 2026 at 3:37 PM Giuseppe Scrivano <giuseppe@scrivano.org> wrote:
>>
>> A metacopy file with an absolute redirect to a data-only layer is followed
>> without metacopy=on and redirect_dir=on only when it is in the last lower
>> layer.  In any other lower layer, the next iteration of the loop in
>> ovl_lookup_layers() calls ovl_check_follow_redirect() with d->metacopy set
>> and fails with -EPERM.
>>
>> A mount with userxattr cannot have metacopy=on and redirect_dir=on, so a
>> user namespace mount that stacks several metadata-only layers over a
>> data-only layer can use the metacopy files of the last lower layer only:
>>
>>   mount -t overlay none -o userxattr,lowerdir+=l3,lowerdir+=l2,\
>>         lowerdir+=l1,datadir+=data,upperdir=u,workdir=w m
>>   cat m/low   # metacopy file in l1: works
>>   cat m/mid   # metacopy file in l2: Operation not permitted
>>
>> Skip the check inside the loop for a metacopy file of a lower layer with an
>> absolute redirect, on a mount that has data-only layers.  The lookup of the
>> data stays lazy and happens only in the data-only layers.  A metacopy file
>> or a redirect in the upper layer is still refused.  With metacopy=on and
>> redirect_dir=on the loop already continues after such a file, so only
>> mounts that failed with -EPERM are affected.
>>
>> Signed-off-by: Giuseppe Scrivano <giuseppe@scrivano.org>
>> ---
>>  fs/overlayfs/namei.c | 13 +++++++++++--
>>  1 file changed, 11 insertions(+), 2 deletions(-)
>>
>> diff --git a/fs/overlayfs/namei.c b/fs/overlayfs/namei.c
>> index ca899fdfaafd..02c64a5652a0 100644
>> --- a/fs/overlayfs/namei.c
>> +++ b/fs/overlayfs/namei.c
>> @@ -1087,6 +1087,14 @@ struct ovl_lookup_ctx {
>>         unsigned int ctr;
>>  };
>>
>> +/* A metacopy file of a lower layer whose lowerdata is in a data-only layer */
>> +static bool ovl_is_datalayer_redirect(struct ovl_lookup_ctx *ctx,
>> +                                     struct ovl_lookup_data *d)
>> +{
>> +       return d->metacopy && ctx->ctr && OVL_FS(d->sb)->numdatalayer &&
>> +              d->absolute_redirect;
>
> To further reduce risk, assuming that in the intended use case the data
> references always carry a digest, can we reinforce this to
> d->metacopy > OVL_METACOPY_MIN_SIZE?

I am not planning to use it only with fsverity.  I'd like to use this
also on file systems where fs-verity is not supported, or if an user
decides to opt-out for any reason.

>
>> +}
>> +
>>  static int ovl_lookup_layers(struct ovl_lookup_ctx *ctx, struct ovl_lookup_data *d)
>>  {
>>         struct dentry *dentry = ctx->dentry;
>> @@ -1154,7 +1162,8 @@ static int ovl_lookup_layers(struct ovl_lookup_ctx *ctx, struct ovl_lookup_data
>>         for (i = 0; !d->stop && i < ovl_numlower(poe); i++) {
>>                 struct ovl_path lower = ovl_lowerstack(poe)[i];
>>
>> -               if (!ovl_check_follow_redirect(d)) {
>> +               if (!ovl_is_datalayer_redirect(ctx, d) &&
>> +                   !ovl_check_follow_redirect(d)) {
>>                         err = -EPERM;
>>                         return err;
>>                 }
>
> That's too promiscuous IMO to continue to loop as if nothing.
> You subject says "always follow metacopy into data-only layers"
> but this allows also following metacopy from l2 to l1 if there is a
> datalower and that makes no sense.
>
> What you would need to do is taint the lookup ctx in case we decided to
> follow against the follow_redirect/metacopy config in such a way
> that finding anything in the lower layers, even a whiteout or opaque
> object, will fail the lookup, because then we are not really allowing
> the intended follow into data-only layers.
>
> But I prefer to do it differently, see below
>
>> @@ -1238,7 +1247,7 @@ static int ovl_lookup_layers(struct ovl_lookup_ctx *ctx, struct ovl_lookup_data
>>          * Defer lookup of lowerdata in data-only layers to first access.
>>          * Don't require redirect=follow and metacopy=on in this case.
>>          */
>> -       if (d->metacopy && ctx->ctr && ofs->numdatalayer && d->absolute_redirect) {
>> +       if (ovl_is_datalayer_redirect(ctx, d)) {
>>                 d->metacopy = 0;
>>                 ctx->ctr++;
>>         } else if (!ovl_check_follow_redirect(d)) {
>> --
>> 2.56.0
>>
>>
>
> If the tooling can be changed we could introduce a special data-only
> follow marker and in this case we could just break out of the layers
> loop when it is encountered.
>
> Another option is to implement opt-in to redirect_dir=followdata
> which only allows following absolute redirects to the data-only layers
> which will stop the layers loop on first found redirect and requires
> a data-only layer of course.
>
> I prefer this option if it meets the user case's needs,
> because verifying that the redirect doesn't redirect to a lower layer
> just complicates the code for a corner case and runs unneeded
> cycles for no good reason.

from an initial test, this seems to work for me.  If I don't encounter
other issues, I'll send another version later.

Thanks for your help!

Giuseppe