[PATCH] [v2] ext4: fix ext4_tune_sb_params padding

Arnd Bergmann posted 1 patch 2 weeks ago
include/uapi/linux/ext4.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[PATCH] [v2] ext4: fix ext4_tune_sb_params padding
Posted by Arnd Bergmann 2 weeks ago
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
Re: [PATCH] [v2] ext4: fix ext4_tune_sb_params padding
Posted by Darrick J. Wong 1 week, 3 days ago
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
>
Re: [PATCH] [v2] ext4: fix ext4_tune_sb_params padding
Posted by Jan Kara 1 week, 3 days ago
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