include/uapi/linux/ext4.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
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.5On 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
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
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
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
> 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
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
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
© 2016 - 2025 Red Hat, Inc.