linux-kernel - Re: [PATCH v6 5/9] ovl: Ensure that all layers have the same encoding

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <bcc8ad0c-669f-4f0d-a795-ec55bb498ef7@igalia.com>
Date: Tue, 26 Aug 2025 17:12:43 -0300
From: André Almeida <andrealmeid@...lia.com>
To: Amir Goldstein <amir73il@...il.com>,
 Gabriel Krisman Bertazi <gabriel@...sman.be>
Cc: Miklos Szeredi <miklos@...redi.hu>, Theodore Tso <tytso@....edu>,
 linux-unionfs@...r.kernel.org, linux-kernel@...r.kernel.org,
 linux-fsdevel@...r.kernel.org, Alexander Viro <viro@...iv.linux.org.uk>,
 Christian Brauner <brauner@...nel.org>, Jan Kara <jack@...e.cz>,
 kernel-dev@...lia.com
Subject: Re: [PATCH v6 5/9] ovl: Ensure that all layers have the same encoding



Em 25/08/2025 12:32, Amir Goldstein escreveu:
> On Mon, Aug 25, 2025 at 1:17 PM Gabriel Krisman Bertazi
> <gabriel@...sman.be> wrote:
>>
>> André Almeida <andrealmeid@...lia.com> writes:
>>
>>> When merging layers from different filesystems with casefold enabled,
>>> all layers should use the same encoding version and have the same flags
>>> to avoid any kind of incompatibility issues.
>>>
>>> Also, set the encoding and the encoding flags for the ovl super block as
>>> the same as used by the first valid layer.
>>>
>>> Reviewed-by: Amir Goldstein <amir73il@...il.com>
>>> Signed-off-by: André Almeida <andrealmeid@...lia.com>
>>> ---
>>>   fs/overlayfs/super.c | 25 +++++++++++++++++++++++++
>>>   1 file changed, 25 insertions(+)
>>>
>>> diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
>>> index df85a76597e910d00323018f1d2cd720c5db921d..b1dbd3c79961094d00c7f99cc622e515d544d22f 100644
>>> --- a/fs/overlayfs/super.c
>>> +++ b/fs/overlayfs/super.c
>>> @@ -991,6 +991,18 @@ static int ovl_get_data_fsid(struct ovl_fs *ofs)
>>>        return ofs->numfs;
>>>   }
>>>
>>> +/*
>>> + * Set the ovl sb encoding as the same one used by the first layer
>>> + */
>>> +static void ovl_set_encoding(struct super_block *sb, struct super_block *fs_sb)
>>> +{
>>> +#if IS_ENABLED(CONFIG_UNICODE)
>>> +     if (sb_has_encoding(fs_sb)) {
>>> +             sb->s_encoding = fs_sb->s_encoding;
>>> +             sb->s_encoding_flags = fs_sb->s_encoding_flags;
>>> +     }
>>> +#endif
>>> +}
>>>
>>>   static int ovl_get_layers(struct super_block *sb, struct ovl_fs *ofs,
>>>                          struct ovl_fs_context *ctx, struct ovl_layer *layers)
>>> @@ -1024,6 +1036,9 @@ static int ovl_get_layers(struct super_block *sb, struct ovl_fs *ofs,
>>>        if (ovl_upper_mnt(ofs)) {
>>>                ofs->fs[0].sb = ovl_upper_mnt(ofs)->mnt_sb;
>>>                ofs->fs[0].is_lower = false;
>>> +
>>> +             if (ofs->casefold)
>>> +                     ovl_set_encoding(sb, ofs->fs[0].sb);
>>>        }
>>>
>>>        nr_merged_lower = ctx->nr - ctx->nr_data;
>>> @@ -1083,6 +1098,16 @@ static int ovl_get_layers(struct super_block *sb, struct ovl_fs *ofs,
>>>                l->name = NULL;
>>>                ofs->numlayer++;
>>>                ofs->fs[fsid].is_lower = true;
>>> +
>>> +             if (ofs->casefold) {
>>> +                     if (!ovl_upper_mnt(ofs) && !sb_has_encoding(sb))
>>> +                             ovl_set_encoding(sb, ofs->fs[fsid].sb);
>>> +
>>> +                     if (!sb_has_encoding(sb) || !sb_same_encoding(sb, mnt->mnt_sb)) {
>>
>> Minor nit, but isn't the sb_has_encoding()  check redundant here?  sb_same_encoding
>> will check the sb->encoding matches the mnt_sb already.
> 
> Maybe we did something wrong but the intention was:
> If all layers root are casefold disabled (or not supported) then
> a mix of layers with fs of different encoding (and fs with no encoding support)
> is allowed because we take care that all directories are always
> casefold disabled.
> 

We are going to reach this code only if ofs->casefold is true, so that 
means that ovl_dentry_casefolded() was true, and that means that 
sb_has_encoding(dentry->d_sb) is also true... so I think that Gabriel is 
right, if we reach this part of the code, that means that casefold is 
enabled and being used by at least one layer, so we can call 
sb_same_encoding() to check if they are consistent for all layers.

For the case that we don't care about the layers having different 
encoding, the code will already skip this because of if (ofs->casefold)

> Thanks,
> Amir.