Re: [PATCH v5] fs/ntfs3: reject out-of-range evcn in mi_enum_attr()

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

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

> In mi_enum_attr(), the start/end VCN validation for non-resident
> attributes is:
>
> 	if (svcn > evcn + 1) goto out;
>
> When evcn is U64_MAX the "evcn + 1" expression wraps to 0 and any svcn
> passes the check. For evcn values close to U64_MAX (but not equal to it)
> the right-hand side is still a meaningless near-wrap upper bound, so a
> malformed on-disk attribute with svcn == 0 and evcn near U64_MAX can pass
> mi_enum_attr() unrejected.
>
> VCN (virtual cluster number) is a cluster index, so any valid evcn is
> bounded by the volume's total cluster count, which ntfs3 holds in
> sbi->used.bitmap.nbits (set up in ntfs_init_from_boot() before any caller
> of mi_enum_attr() runs). Reject evcn values that fall outside this range.
>
> However, an empty non-resident attribute (no allocated clusters) is
> legitimately encoded with svcn == 0 and evcn == -1 (U64_MAX), e.g. via
> attr->nres.evcn = cpu_to_le64((u64)vcn - 1) with vcn == 0. That sentinel
> must keep passing, so exclude evcn == U64_MAX from the range check. The
> existing "svcn > evcn + 1" test still tolerates the sentinel ("0 > 0" is
> false) and continues to require svcn == 0 for it, while the range check
> rejects every other out-of-range evcn and thereby also defuses the
> "evcn + 1" wraparound.
>
> svcn does not need its own bound: once evcn < nbits, "svcn > evcn + 1"
> implies svcn <= nbits.
>
> Fixes: 013ff63b6494 ("fs/ntfs3: Add more attributes checks in mi_enum_attr()")
> Signed-off-by: Zhan Xusheng <zhanxusheng@xiaomi.com>
> ---
>   fs/ntfs3/record.c | 20 +++++++++++++++++---
>   1 file changed, 17 insertions(+), 3 deletions(-)
>
> diff --git a/fs/ntfs3/record.c b/fs/ntfs3/record.c
> index 32bdb034c2a3..d6ca4ac35a45 100644
> --- a/fs/ntfs3/record.c
> +++ b/fs/ntfs3/record.c
> @@ -202,7 +202,7 @@ struct ATTRIB *mi_enum_attr(struct ntfs_inode *ni, struct mft_inode *mi,
>   	u32 used = le32_to_cpu(rec->used);
>   	u32 t32, off, asize, prev_type;
>   	u16 t16;
> -	u64 data_size, alloc_size, tot_size;
> +	u64 svcn, evcn, data_size, alloc_size, tot_size;
>   
>   	if (!attr) {
>   		u32 total = le32_to_cpu(rec->total);
> @@ -310,8 +310,22 @@ struct ATTRIB *mi_enum_attr(struct ntfs_inode *ni, struct mft_inode *mi,
>   	if (t32 && le16_to_cpu(attr->name_off) + t32 > t16)
>   		goto out;
>   
> -	/* Check start/end vcn. */
> -	if (le64_to_cpu(attr->nres.svcn) > le64_to_cpu(attr->nres.evcn) + 1)
> +	/*
> +	 * Check start/end vcn.  svcn == 0 with evcn == -1 (U64_MAX) is the
> +	 * sentinel for an empty non-resident attribute (no allocated
> +	 * clusters) and must be accepted: "svcn > evcn + 1" tolerates it,
> +	 * since "(u64)-1 + 1" is 0 and "0 > 0" is false.
> +	 *
> +	 * For a non-empty attribute evcn is a cluster index and must lie
> +	 * within the volume (sbi->used.bitmap.nbits, set up in
> +	 * ntfs_init_from_boot() before any caller of mi_enum_attr() runs).
> +	 * Bounding evcn also prevents a malformed value close to U64_MAX
> +	 * from slipping through the near-wrap "evcn + 1" upper bound.
> +	 */
> +	svcn = le64_to_cpu(attr->nres.svcn);
> +	evcn = le64_to_cpu(attr->nres.evcn);
> +	if (svcn > evcn + 1 ||
> +	    (evcn != U64_MAX && evcn >= mi->sbi->used.bitmap.nbits))
>   		goto out;
>   
>   	data_size = le64_to_cpu(attr->nres.data_size);

Hello,

Sorry for the delay.
Your patch was applied, thanks.

Regards,
Konstantin