[PATCH] ext4: fix ext4_tune_sb_params padding

[PATCH] ext4: fix ext4_tune_sb_params padding

[Arnd Bergmann]

From: Arnd Bergmann <arnd@arndb.de>

The padding at the end of struct ext4_tune_sb_params is architecture
specific and in particular is different between x86-32 and x86-64,
since the __u64 member only enforces struct alignment on the latter.

This shows up as a new warning when test-building the headers with
-Wpadded:

include/linux/ext4.h:144:1: error: padding struct size to alignment boundary with 4 bytes [-Werror=padded]

All members inside the structure are naturally aligned, so the only
difference here is the amount of padding at the end. Make the padding
explicit, to have a consistent sizeof(struct ext4_tune_sb_params) of
232 on all architectures and avoid adding compat ioctl handling for
EXT4_IOC_GET_TUNE_SB_PARAM/EXT4_IOC_SET_TUNE_SB_PARAM.

This is an ABI break on x86-32 but hopefully this can go into 6.18.y early
enough as a fixup so no actual users will be affected.  Alternatively, the
kernel could handle the ioctl commands for both sizes (232 and 228 bytes)
on all architectures.

Fixes: 04a91570ac67 ("ext4: implemet new ioctls to set and get superblock parameters")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 include/uapi/linux/ext4.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/uapi/linux/ext4.h b/include/uapi/linux/ext4.h
index 411dcc1e4a35..9c683991c32f 100644
--- a/include/uapi/linux/ext4.h
+++ b/include/uapi/linux/ext4.h
@@ -139,7 +139,7 @@ struct ext4_tune_sb_params {
 	__u32 clear_feature_incompat_mask;
 	__u32 clear_feature_ro_compat_mask;
 	__u8  mount_opts[64];
-	__u8  pad[64];
+	__u8  pad[68];
 };
 
 #define EXT4_TUNE_FL_ERRORS_BEHAVIOR	0x00000001
-- 
2.39.5

Re: [PATCH] ext4: fix ext4_tune_sb_params padding

[Jan Kara]

On Thu 04-12-25 11:19:10, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> The padding at the end of struct ext4_tune_sb_params is architecture
> specific and in particular is different between x86-32 and x86-64,
> since the __u64 member only enforces struct alignment on the latter.
> 
> This shows up as a new warning when test-building the headers with
> -Wpadded:
> 
> include/linux/ext4.h:144:1: error: padding struct size to alignment boundary with 4 bytes [-Werror=padded]
> 
> All members inside the structure are naturally aligned, so the only
> difference here is the amount of padding at the end. Make the padding
> explicit, to have a consistent sizeof(struct ext4_tune_sb_params) of
> 232 on all architectures and avoid adding compat ioctl handling for
> EXT4_IOC_GET_TUNE_SB_PARAM/EXT4_IOC_SET_TUNE_SB_PARAM.
> 
> This is an ABI break on x86-32 but hopefully this can go into 6.18.y early
> enough as a fixup so no actual users will be affected.  Alternatively, the
> kernel could handle the ioctl commands for both sizes (232 and 228 bytes)
> on all architectures.
> 
> Fixes: 04a91570ac67 ("ext4: implemet new ioctls to set and get superblock parameters")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Indeed. I agree this is fairly new so we can just fix the structure. Feel
free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  include/uapi/linux/ext4.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/uapi/linux/ext4.h b/include/uapi/linux/ext4.h
> index 411dcc1e4a35..9c683991c32f 100644
> --- a/include/uapi/linux/ext4.h
> +++ b/include/uapi/linux/ext4.h
> @@ -139,7 +139,7 @@ struct ext4_tune_sb_params {
>  	__u32 clear_feature_incompat_mask;
>  	__u32 clear_feature_ro_compat_mask;
>  	__u8  mount_opts[64];
> -	__u8  pad[64];
> +	__u8  pad[68];
>  };
>  
>  #define EXT4_TUNE_FL_ERRORS_BEHAVIOR	0x00000001
> -- 
> 2.39.5
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH] ext4: fix ext4_tune_sb_params padding

[David Laight]

On Thu,  4 Dec 2025 11:19:10 +0100
Arnd Bergmann <arnd@kernel.org> wrote:

> From: Arnd Bergmann <arnd@arndb.de>
> 
> The padding at the end of struct ext4_tune_sb_params is architecture
> specific and in particular is different between x86-32 and x86-64,
> since the __u64 member only enforces struct alignment on the latter.

Is it worth adding a compile-time check for the size somewhere?
Since the intention seems to be that any extensions will use the padding.

	David

> 
> This shows up as a new warning when test-building the headers with
> -Wpadded:
> 
> include/linux/ext4.h:144:1: error: padding struct size to alignment boundary with 4 bytes [-Werror=padded]
> 
> All members inside the structure are naturally aligned, so the only
> difference here is the amount of padding at the end. Make the padding
> explicit, to have a consistent sizeof(struct ext4_tune_sb_params) of
> 232 on all architectures and avoid adding compat ioctl handling for
> EXT4_IOC_GET_TUNE_SB_PARAM/EXT4_IOC_SET_TUNE_SB_PARAM.
> 
> This is an ABI break on x86-32 but hopefully this can go into 6.18.y early
> enough as a fixup so no actual users will be affected.  Alternatively, the
> kernel could handle the ioctl commands for both sizes (232 and 228 bytes)
> on all architectures.
> 
> Fixes: 04a91570ac67 ("ext4: implemet new ioctls to set and get superblock parameters")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  include/uapi/linux/ext4.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/uapi/linux/ext4.h b/include/uapi/linux/ext4.h
> index 411dcc1e4a35..9c683991c32f 100644
> --- a/include/uapi/linux/ext4.h
> +++ b/include/uapi/linux/ext4.h
> @@ -139,7 +139,7 @@ struct ext4_tune_sb_params {
>  	__u32 clear_feature_incompat_mask;
>  	__u32 clear_feature_ro_compat_mask;
>  	__u8  mount_opts[64];
> -	__u8  pad[64];
> +	__u8  pad[68];
>  };
>  
>  #define EXT4_TUNE_FL_ERRORS_BEHAVIOR	0x00000001

Re: [PATCH] ext4: fix ext4_tune_sb_params padding

[Arnd Bergmann]

On Thu, Dec 4, 2025, at 13:35, David Laight wrote:
> On Thu,  4 Dec 2025 11:19:10 +0100
> Arnd Bergmann <arnd@kernel.org> wrote:
>
>> From: Arnd Bergmann <arnd@arndb.de>
>> 
>> The padding at the end of struct ext4_tune_sb_params is architecture
>> specific and in particular is different between x86-32 and x86-64,
>> since the __u64 member only enforces struct alignment on the latter.
>
> Is it worth adding a compile-time check for the size somewhere?
> Since the intention seems to be that any extensions will use the padding.

There is already ABI checking with abigail that ensures that struct
members and sizes don't change in the future, which I think covers
that. I would also like to push my series to enable -Werror=padded
in the header checks, but I'm not sure yet what others think of the
idea.

     Arnd

Re: [PATCH] ext4: fix ext4_tune_sb_params padding

[David Laight]

On Thu, 04 Dec 2025 14:42:06 +0100
"Arnd Bergmann" <arnd@arndb.de> wrote:

> On Thu, Dec 4, 2025, at 13:35, David Laight wrote:
> > On Thu,  4 Dec 2025 11:19:10 +0100
> > Arnd Bergmann <arnd@kernel.org> wrote:
> >  
> >> From: Arnd Bergmann <arnd@arndb.de>
> >> 
> >> The padding at the end of struct ext4_tune_sb_params is architecture
> >> specific and in particular is different between x86-32 and x86-64,
> >> since the __u64 member only enforces struct alignment on the latter.  
> >
> > Is it worth adding a compile-time check for the size somewhere?
> > Since the intention seems to be that any extensions will use the padding.  
> 
> There is already ABI checking with abigail that ensures that struct
> members and sizes don't change in the future, which I think covers
> that. I would also like to push my series to enable -Werror=padded
> in the header checks, but I'm not sure yet what others think of the
> idea.

Putting it in the command line is going to be griefsome (at least in the
short term) even for uapi headers - where you really don't want padding.
(Tell that to some of the standards bodies...)
It is a shame there isn't an attribute, but you can wrap definitions:

#define check_padding(...) _Pragma("GCC diagnostic push"); \
    _Pragma("GCC diagnostic error \"-Wpadded\""); \
    __VA_ARGS__ \
    _Pragma("GCC diagnostic pop");
    
check_padding(

typedef struct fubar {
    int a;
    char b;
} fred;

) /* check_padding */

I've thought about doing something similar to avoid the 'type-limits' check
inside statically_true() and the like for W=1 builds.

	David

Re: [PATCH] ext4: fix ext4_tune_sb_params padding

[Andreas Dilger]

> On Dec 4, 2025, at 3:31 AM, Jan Kara <jack@suse.cz> wrote:
> 
> On Thu 04-12-25 11:19:10, Arnd Bergmann wrote:
>> From: Arnd Bergmann <arnd@arndb.de>
>> 
>> The padding at the end of struct ext4_tune_sb_params is architecture
>> specific and in particular is different between x86-32 and x86-64,
>> since the __u64 member only enforces struct alignment on the latter.
>> 
>> This shows up as a new warning when test-building the headers with
>> -Wpadded:
>> 
>> include/linux/ext4.h:144:1: error: padding struct size to alignment boundary with 4 bytes [-Werror=padded]
>> 
>> All members inside the structure are naturally aligned, so the only
>> difference here is the amount of padding at the end. Make the padding
>> explicit, to have a consistent sizeof(struct ext4_tune_sb_params) of
>> 232 on all architectures and avoid adding compat ioctl handling for
>> EXT4_IOC_GET_TUNE_SB_PARAM/EXT4_IOC_SET_TUNE_SB_PARAM.
>> 
>> This is an ABI break on x86-32 but hopefully this can go into 6.18.y early
>> enough as a fixup so no actual users will be affected.  Alternatively, the
>> kernel could handle the ioctl commands for both sizes (232 and 228 bytes)
>> on all architectures.
>> 
>> Fixes: 04a91570ac67 ("ext4: implemet new ioctls to set and get superblock parameters")
>> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> 
> Indeed. I agree this is fairly new so we can just fix the structure. Feel
> free to add:
> 
> Reviewed-by: Jan Kara <jack@suse.cz>

While this change isn't _wrong_ per-se, it does seem very strange to have
a 68-byte padding at the end of the struct.  You have to check the number
of __u32 fields closely to see this, and I wonder if this will perpetuate
errors in the future (e.g. adding a __u64 field after mount_opts[64]).

IMHO, it would be more clear to either add an explicit "__u32 pad_3;"
field after mount_opts[64], or alternately declare mount_opts[68] so it
will consume those bytes and leave the remaining fields properly aligned.
It isn't critical if the user tools use the last 4 bytes of mount_opts[]
or not, so they could be changed independently at some later time.

Either will ensure that new fields added in place of pad[64] will be
properly aligned in the future.

Cheers, Andreas


> 
> Honza
> 
>> ---
>> include/uapi/linux/ext4.h | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/include/uapi/linux/ext4.h b/include/uapi/linux/ext4.h
>> index 411dcc1e4a35..9c683991c32f 100644
>> --- a/include/uapi/linux/ext4.h
>> +++ b/include/uapi/linux/ext4.h
>> @@ -139,7 +139,7 @@ struct ext4_tune_sb_params {
>> __u32 clear_feature_incompat_mask;
>> __u32 clear_feature_ro_compat_mask;
>> __u8  mount_opts[64];
>> - __u8  pad[64];
>> + __u8  pad[68];
>> };
>> 
>> #define EXT4_TUNE_FL_ERRORS_BEHAVIOR 0x00000001
>> -- 
>> 2.39.5
>> 
> -- 
> Jan Kara <jack@suse.com>
> SUSE Labs, CR



Cheers, Andreas

Re: [PATCH] ext4: fix ext4_tune_sb_params padding

[Arnd Bergmann]

On Fri, Dec 5, 2025, at 11:17, Andreas Dilger wrote:
>> On Dec 4, 2025, at 3:31 AM, Jan Kara <jack@suse.cz> wrote:
>> On Thu 04-12-25 11:19:10, Arnd Bergmann wrote:
>
> While this change isn't _wrong_ per-se, it does seem very strange to have
> a 68-byte padding at the end of the struct.  You have to check the number
> of __u32 fields closely to see this, 

I had the same thought but decided against that because it would be
an ABI break on all architectures. The version I posted only changes
the structure size on x86-32, csky, m68k and microblaze, as far
as I can tell.

> and I wonder if this will perpetuate
> errors in the future (e.g. adding a __u64 field after mount_opts[64]).

Indeed, I can see how that could become worse.

> IMHO, it would be more clear to either add an explicit "__u32 pad_3;"
> field after mount_opts[64], or alternately declare mount_opts[68] so it
> will consume those bytes and leave the remaining fields properly aligned.
> It isn't critical if the user tools use the last 4 bytes of mount_opts[]
> or not, so they could be changed independently at some later time.
>
> Either will ensure that new fields added in place of pad[64] will be
> properly aligned in the future.

Changing mount_opts[] to 68 bytes sounds fine to me, I'll send an
updated patch for that. I've kept the Ack from Jan, please shout
if I should drop that instead.

   Arnd

Re: [PATCH] ext4: fix ext4_tune_sb_params padding

[Geert Uytterhoeven]

On Fri, 5 Dec 2025 at 19:07, Arnd Bergmann <arnd@arndb.de> wrote:
> On Fri, Dec 5, 2025, at 11:17, Andreas Dilger wrote:
> >> On Dec 4, 2025, at 3:31 AM, Jan Kara <jack@suse.cz> wrote:
> >> On Thu 04-12-25 11:19:10, Arnd Bergmann wrote:
> > While this change isn't _wrong_ per-se, it does seem very strange to have
> > a 68-byte padding at the end of the struct.  You have to check the number
> > of __u32 fields closely to see this,
>
> I had the same thought but decided against that because it would be
> an ABI break on all architectures. The version I posted only changes
> the structure size on x86-32, csky, m68k and microblaze, as far
> as I can tell.

Indeed very unfortunate...

> > and I wonder if this will perpetuate
> > errors in the future (e.g. adding a __u64 field after mount_opts[64]).
>
> Indeed, I can see how that could become worse.
>
> > IMHO, it would be more clear to either add an explicit "__u32 pad_3;"
> > field after mount_opts[64], or alternately declare mount_opts[68] so it

FTR, I would have added "__u32 pad_3;" _before_ mount_opts[64].

> > will consume those bytes and leave the remaining fields properly aligned.
> > It isn't critical if the user tools use the last 4 bytes of mount_opts[]
> > or not, so they could be changed independently at some later time.
> >
> > Either will ensure that new fields added in place of pad[64] will be
> > properly aligned in the future.
>
> Changing mount_opts[] to 68 bytes sounds fine to me, I'll send an
> updated patch for that. I've kept the Ack from Jan, please shout
> if I should drop that instead.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] ext4: fix ext4_tune_sb_params padding

[Theodore Ts'o]

On Thu, 04 Dec 2025 11:19:10 +0100, Arnd Bergmann wrote:
> The padding at the end of struct ext4_tune_sb_params is architecture
> specific and in particular is different between x86-32 and x86-64,
> since the __u64 member only enforces struct alignment on the latter.
> 
> This shows up as a new warning when test-building the headers with
> -Wpadded:
> 
> [...]

Applied, thanks!

[1/1] ext4: fix ext4_tune_sb_params padding
      (no commit info)

Best regards,
-- 
Theodore Ts'o <tytso@mit.edu>