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

André Almeida posted 9 patches 1 month, 1 week ago
[PATCH v6 5/9] ovl: Ensure that all layers have the same encoding
Posted by André Almeida 1 month, 1 week ago
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@gmail.com>
Signed-off-by: André Almeida <andrealmeid@igalia.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)) {
+				pr_err("all layers must have the same encoding\n");
+				return -EINVAL;
+			}
+		}
 	}
 
 	/*

-- 
2.50.1

Re: [PATCH v6 5/9] ovl: Ensure that all layers have the same encoding
Posted by Gabriel Krisman Bertazi 1 month, 1 week ago
André Almeida <andrealmeid@igalia.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@gmail.com>
> Signed-off-by: André Almeida <andrealmeid@igalia.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.

> +				pr_err("all layers must have the same encoding\n");
> +				return -EINVAL;
> +			}
> +		}
>  	}
>
>  	/*

-- 
Gabriel Krisman Bertazi
Re: [PATCH v6 5/9] ovl: Ensure that all layers have the same encoding
Posted by Amir Goldstein 1 month, 1 week ago
On Mon, Aug 25, 2025 at 1:17 PM Gabriel Krisman Bertazi
<gabriel@krisman.be> wrote:
>
> André Almeida <andrealmeid@igalia.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@gmail.com>
> > Signed-off-by: André Almeida <andrealmeid@igalia.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.

Thanks,
Amir.
Re: [PATCH v6 5/9] ovl: Ensure that all layers have the same encoding
Posted by André Almeida 1 month, 1 week ago

Em 25/08/2025 12:32, Amir Goldstein escreveu:
> On Mon, Aug 25, 2025 at 1:17 PM Gabriel Krisman Bertazi
> <gabriel@krisman.be> wrote:
>>
>> André Almeida <andrealmeid@igalia.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@gmail.com>
>>> Signed-off-by: André Almeida <andrealmeid@igalia.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.

Re: [PATCH v6 5/9] ovl: Ensure that all layers have the same encoding
Posted by Amir Goldstein 1 month, 1 week ago
On Tue, Aug 26, 2025 at 10:12 PM André Almeida <andrealmeid@igalia.com> wrote:
>
>
>
> Em 25/08/2025 12:32, Amir Goldstein escreveu:
> > On Mon, Aug 25, 2025 at 1:17 PM Gabriel Krisman Bertazi
> > <gabriel@krisman.be> wrote:
> >>
> >> André Almeida <andrealmeid@igalia.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@gmail.com>
> >>> Signed-off-by: André Almeida <andrealmeid@igalia.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)

Doh! yeh that was silly. I removed that now.

Thanks,
Amir.