Re: [PATCH] fs/ntfs3: fix integer overflow in MFT cluster validation

[Konstantin Komarov <almaz.alexandrovich@paragon-software.com>]

On 6/24/26 05:41, Zhan Xusheng wrote:

> In ntfs_init_from_boot(), the boot sector's MFT cluster numbers are
> validated against the volume size with:
>
> 	if (mlcn  * sct_per_clst >= sectors ||
> 	    mlcn2 * sct_per_clst >= sectors)
> 		goto out;
>
> mlcn and mlcn2 are u64 fields read directly from the boot sector.
> sct_per_clst is bounded above by 4096 (true_sectors_per_clst() plus
> the is_power_of_2() check below it), but the multiplication is done
> in u64 and wraps when mlcn (or mlcn2) is large enough -- e.g. mlcn
> near 2^62 with sct_per_clst == 4 wraps to 0, which compares below
> any non-zero 'sectors', so the check is bypassed and the malformed
> record is accepted.
>
> The accepted mlcn is then used unchanged in
>
> 	sbi->mft.lbo = mlcn << cluster_bits;
>
> In practice the resulting reads fail at the block layer (sb_bread()
> returns NULL via grow_buffers()'s check_mul_overflow() guard), so
> today this manifests as mount failing in odd places rather than as
> something more dangerous, but the validation step is still wrong
> and there is no reason for callers to rely on the block layer to
> catch a value that should never have been accepted in the first
> place.
>
> Use check_mul_overflow() to compute the two sector positions and
> fail the mount if either multiplication wraps; this preserves the
> existing semantics (mlcn * sct_per_clst >= sectors) instead of
> switching to division (mlcn >= sectors / sct_per_clst), which
> would tighten the check at edge cases where 'sectors' is not a
> multiple of sct_per_clst.  The check_*_overflow() style is the
> one ntfs3 already uses for similar on-disk arithmetic in
> fs/ntfs3/run.c.
>
> Fixes: 82cae269cfa9 ("fs/ntfs3: Add initialization of super block")
> Signed-off-by: Zhan Xusheng <zhanxusheng@xiaomi.com>
> ---
>   fs/ntfs3/super.c | 13 +++++++++++--
>   1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/fs/ntfs3/super.c b/fs/ntfs3/super.c
> index 3305fe406cb2..4205a212154b 100644
> --- a/fs/ntfs3/super.c
> +++ b/fs/ntfs3/super.c
> @@ -65,6 +65,7 @@
>   #include <linux/minmax.h>
>   #include <linux/module.h>
>   #include <linux/nls.h>
> +#include <linux/overflow.h>
>   #include <linux/proc_fs.h>
>   #include <linux/seq_file.h>
>   #include <linux/statfs.h>
> @@ -957,7 +958,7 @@ static int ntfs_init_from_boot(struct super_block *sb, u32 sector_size,
>   	struct ntfs_sb_info *sbi = sb->s_fs_info;
>   	int err;
>   	u32 mb, gb, boot_sector_size, sct_per_clst, record_size;
> -	u64 sectors, clusters, mlcn, mlcn2, dev_size0;
> +	u64 sectors, clusters, mlcn, mlcn2, mft_pos, mft2_pos, dev_size0;
>   	struct NTFS_BOOT *boot;
>   	struct buffer_head *bh;
>   	struct MFT_REC *rec;
> @@ -1026,7 +1027,15 @@ static int ntfs_init_from_boot(struct super_block *sb, u32 sector_size,
>   	mlcn2 = le64_to_cpu(boot->mft2_clst);
>   	sectors = le64_to_cpu(boot->sectors_per_volume);
>   
> -	if (mlcn * sct_per_clst >= sectors || mlcn2 * sct_per_clst >= sectors) {
> +	/*
> +	 * Convert mlcn/mlcn2 to sector positions before comparing with
> +	 * 'sectors'.  All three are u64 values that come from the boot
> +	 * sector, so use check_mul_overflow() to keep a wraparound from
> +	 * silently bypassing the comparison.
> +	 */
> +	if (check_mul_overflow(mlcn, (u64)sct_per_clst, &mft_pos) ||
> +	    check_mul_overflow(mlcn2, (u64)sct_per_clst, &mft2_pos) ||
> +	    mft_pos >= sectors || mft2_pos >= sectors) {
>   		ntfs_err(
>   			sb,
>   			"%s: start of MFT 0x%llx (0x%llx) is out of volume 0x%llx.",

Hello,

Sorry for the delay.
Your patch was applied, thank you.

Regards,
Konstantin