Re: [PATCH] ext4: don't read past the UAPI struct in EXT4_IOC_GROUP_ADD

[sashiko-bot@kernel.org]

> From: Kaixuan Li <kaixuanli0131@gmail.com>
> 
> EXT4_IOC_GROUP_ADD casts the user pointer to struct ext4_new_group_input,
> its UAPI type, but sizes the copy by struct ext4_new_group_data, the
> internal type:
> 
> 	struct ext4_new_group_data input;
> 
> 	if (copy_from_user(&input, (struct ext4_new_group_input __user *)arg,
> 			sizeof(input)))
> 
> The UAPI struct is 40 bytes, the internal one 48.  A caller that follows
> the UAPI and allocates 40 bytes gets 8 bytes read past its object, and the
> ioctl returns EFAULT when those bytes are unmapped.  The internal struct
> has carried the extra fields since ext4 was split from ext3, so the copy
> has always over-read the UAPI object.
> 
> The two extra fields are not used from this path: free_clusters_count is
> overwritten by verify_group_input(), and mdata_blocks is used only by
> ext4_resize_fs(), which builds its own group_data array.  The compat path
> already copies the six UAPI fields individually; the native path does not.
> 
> Copy the UAPI struct, then set the internal fields from it.
> 
> Reproducible on v6.12.9 and v7.2.4 by placing a 40-byte struct at the end
> of a mapped page whose successor is unmapped: the ioctl returns EFAULT.
> 
> Signed-off-by: Kaixuan Li <kaixuanli0131@gmail.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260917151949.415967-1-kaixuan.li@ntu.edu.sg?part=1