* [PATCH] [v2] ext4: fix ext4_tune_sb_params padding
@ 2025-12-05 11:19 Arnd Bergmann
2025-12-08 18:03 ` Darrick J. Wong
0 siblings, 1 reply; 4+ messages in thread
From: Arnd Bergmann @ 2025-12-05 11:19 UTC (permalink / raw)
To: Theodore Ts'o, Andreas Dilger, Jan Kara, Darrick J. Wong
Cc: Arnd Bergmann, linux-ext4, linux-kernel
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.
Add explicit padding to mount_opts[] to keep the struct members compatible
with the original version and also keep the pad[64] member 8-byte
aligned for future extensions. This gives a consistent sizeof(struct
ext4_tune_sb_params) of 232 on all architectures and avoids 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.
Fixes: 04a91570ac67 ("ext4: implemet new ioctls to set and get superblock parameters")
Reviewed-by: Jan Kara <jack@suse.cz>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
v2: extend mount_opts[] instead of pad[], as suggested by Andreas Dilger
---
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 6829d6f1497d..1c7cdcdb7dca 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_compat_mask;
__u32 clear_feature_incompat_mask;
__u32 clear_feature_ro_compat_mask;
- __u8 mount_opts[64];
+ __u8 mount_opts[68];
__u8 pad[64];
};
--
2.39.5
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] [v2] ext4: fix ext4_tune_sb_params padding
2025-12-05 11:19 [PATCH] [v2] ext4: fix ext4_tune_sb_params padding Arnd Bergmann
@ 2025-12-08 18:03 ` Darrick J. Wong
2025-12-09 8:56 ` Jan Kara
2026-01-08 4:12 ` Theodore Tso
0 siblings, 2 replies; 4+ messages in thread
From: Darrick J. Wong @ 2025-12-08 18:03 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Theodore Ts'o, Andreas Dilger, Jan Kara, Arnd Bergmann,
linux-ext4, linux-kernel
On Fri, Dec 05, 2025 at 12:19:00PM +0100, 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.
>
> Add explicit padding to mount_opts[] to keep the struct members compatible
> with the original version and also keep the pad[64] member 8-byte
> aligned for future extensions. This gives a consistent sizeof(struct
> ext4_tune_sb_params) of 232 on all architectures and avoids 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.
>
> Fixes: 04a91570ac67 ("ext4: implemet new ioctls to set and get superblock parameters")
> Reviewed-by: Jan Kara <jack@suse.cz>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> v2: extend mount_opts[] instead of pad[], as suggested by Andreas Dilger
> ---
> 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 6829d6f1497d..1c7cdcdb7dca 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_compat_mask;
> __u32 clear_feature_incompat_mask;
> __u32 clear_feature_ro_compat_mask;
> - __u8 mount_opts[64];
> + __u8 mount_opts[68];
Hmm... given that the ondisk super field is a __u8[64], it feels weird
to expose a __u8[68] field in the ioctl ABI and silently truncate the
user's input if they try to use that many bytes. I'd have enlarged the
padding field but as Ted was both author and maintainer I'm ok with
letting him have the final say.
--D
> __u8 pad[64];
> };
>
> --
> 2.39.5
>
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] [v2] ext4: fix ext4_tune_sb_params padding
2025-12-08 18:03 ` Darrick J. Wong
@ 2025-12-09 8:56 ` Jan Kara
2026-01-08 4:12 ` Theodore Tso
1 sibling, 0 replies; 4+ messages in thread
From: Jan Kara @ 2025-12-09 8:56 UTC (permalink / raw)
To: Darrick J. Wong
Cc: Arnd Bergmann, Theodore Ts'o, Andreas Dilger, Jan Kara,
Arnd Bergmann, linux-ext4, linux-kernel
On Mon 08-12-25 10:03:10, Darrick J. Wong wrote:
> On Fri, Dec 05, 2025 at 12:19:00PM +0100, 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.
> >
> > Add explicit padding to mount_opts[] to keep the struct members compatible
> > with the original version and also keep the pad[64] member 8-byte
> > aligned for future extensions. This gives a consistent sizeof(struct
> > ext4_tune_sb_params) of 232 on all architectures and avoids 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.
> >
> > Fixes: 04a91570ac67 ("ext4: implemet new ioctls to set and get superblock parameters")
> > Reviewed-by: Jan Kara <jack@suse.cz>
> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> > ---
> > v2: extend mount_opts[] instead of pad[], as suggested by Andreas Dilger
> > ---
> > 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 6829d6f1497d..1c7cdcdb7dca 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_compat_mask;
> > __u32 clear_feature_incompat_mask;
> > __u32 clear_feature_ro_compat_mask;
> > - __u8 mount_opts[64];
> > + __u8 mount_opts[68];
>
> Hmm... given that the ondisk super field is a __u8[64], it feels weird
> to expose a __u8[68] field in the ioctl ABI and silently truncate the
> user's input if they try to use that many bytes. I'd have enlarged the
> padding field but as Ted was both author and maintainer I'm ok with
> letting him have the final say.
The point about silent truncation is a good one. I thought
ext4_sb_setparams() checks the return value of strscpy_pad() but it doesn't
so IMO that needs fixing.
Honza
>
> --D
>
> > __u8 pad[64];
> > };
> >
> > --
> > 2.39.5
> >
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] [v2] ext4: fix ext4_tune_sb_params padding
2025-12-08 18:03 ` Darrick J. Wong
2025-12-09 8:56 ` Jan Kara
@ 2026-01-08 4:12 ` Theodore Tso
1 sibling, 0 replies; 4+ messages in thread
From: Theodore Tso @ 2026-01-08 4:12 UTC (permalink / raw)
To: Darrick J. Wong
Cc: Arnd Bergmann, Andreas Dilger, Jan Kara, Arnd Bergmann,
linux-ext4, linux-kernel
On Mon, Dec 08, 2025 at 10:03:10AM -0800, Darrick J. Wong wrote:
>
> Hmm... given that the ondisk super field is a __u8[64], it feels weird
> to expose a __u8[68] field in the ioctl ABI and silently truncate the
> user's input if they try to use that many bytes. I'd have enlarged the
> padding field but as Ted was both author and maintainer I'm ok with
> letting him have the final say.
Thanks, I've elected to the the original v1 version of the patch.
- Ted
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-01-08 4:16 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-12-05 11:19 [PATCH] [v2] ext4: fix ext4_tune_sb_params padding Arnd Bergmann
2025-12-08 18:03 ` Darrick J. Wong
2025-12-09 8:56 ` Jan Kara
2026-01-08 4:12 ` Theodore Tso
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox