Messages in this thread |  | | Date | Tue, 9 Dec 2025 09:56:25 +0100 | | From | Jan Kara <> | | Subject | Re: [PATCH] [v2] ext4: fix ext4_tune_sb_params padding |
| |
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
|  |