linux-kernel - Re: [PATCH] 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] [thread-next>] [day] [month] [year] [list]

Message-Id: <B2AC14DC-0B9D-433A-A1B0-78D0778D0A39@dilger.ca>
Date: Fri, 5 Dec 2025 03:17:28 -0700
From: Andreas Dilger <adilger@...ger.ca>
To: Jan Kara <jack@...e.cz>,
 Arnd Bergmann <arnd@...nel.org>
Cc: Theodore Ts'o <tytso@....edu>,
 "Darrick J. Wong" <djwong@...nel.org>,
 Arnd Bergmann <arnd@...db.de>,
 linux-ext4@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH] ext4: fix ext4_tune_sb_params padding

> On Dec 4, 2025, at 3:31 AM, Jan Kara <jack@...e.cz> wrote:
> 
> On Thu 04-12-25 11:19:10, 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. 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@...db.de>
> 
> Indeed. I agree this is fairly new so we can just fix the structure. Feel
> free to add:
> 
> Reviewed-by: Jan Kara <jack@...e.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@...e.com>
> SUSE Labs, CR

Cheers, Andreas