linux-kernel - Re: [PATCH] [v2] ext4: fix ext4_tune_sb

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <3d5u34hrtdiyiyqporwsyfde5a5msyodnjw5oulhdib3givjek@aakrdecmyiou>
Date: Tue, 9 Dec 2025 09:56:25 +0100
From: Jan Kara <jack@...e.cz>
To: "Darrick J. Wong" <djwong@...nel.org>
Cc: Arnd Bergmann <arnd@...nel.org>, Theodore Ts'o <tytso@....edu>, 
	Andreas Dilger <adilger.kernel@...ger.ca>, Jan Kara <jack@...e.cz>, Arnd Bergmann <arnd@...db.de>, 
	linux-ext4@...r.kernel.org, linux-kernel@...r.kernel.org
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@...db.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@...e.cz>
> > Signed-off-by: Arnd Bergmann <arnd@...db.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@...e.com>
SUSE Labs, CR