[PATCH] fs/ntfs3: reject evcn == U64_MAX in mi_enum_attr()

[PATCH] fs/ntfs3: reject evcn == U64_MAX in mi_enum_attr()

[Zhan Xusheng]

In mi_enum_attr(), the start/end VCN validation for non-resident
attributes is:

    if (svcn > evcn + 1) goto out;

When on-disk evcn is 0xFFFFFFFFFFFFFFFF (U64_MAX), the addition
evcn + 1 wraps to 0, and the check becomes "if (svcn > 0)" which
incorrectly accepts svcn == 0 with evcn == U64_MAX.

mi_enum_attr() is the core attribute iterator used throughout ntfs3.
Allowing this malformed VCN range to pass the initial validation can
feed a bogus 64-bit extent span into downstream code that computes
evcn + 1 - svcn (producing 0 due to wrap) or uses evcn as a loop
bound.

A crafted NTFS image can trigger this on mount or file access.

Fix by explicitly rejecting evcn == U64_MAX before the addition.

Fixes: 013ff63b6494 ("fs/ntfs3: Add more attributes checks in mi_enum_attr()")
Signed-off-by: Zhan Xusheng <zhanxusheng@xiaomi.com>
---
 fs/ntfs3/record.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/ntfs3/record.c b/fs/ntfs3/record.c
index 32bdb034c2a3..2ff28bfbedad 100644
--- a/fs/ntfs3/record.c
+++ b/fs/ntfs3/record.c
@@ -311,7 +311,8 @@ struct ATTRIB *mi_enum_attr(struct ntfs_inode *ni, struct mft_inode *mi,
 		goto out;
 
 	/* Check start/end vcn. */
-	if (le64_to_cpu(attr->nres.svcn) > le64_to_cpu(attr->nres.evcn) + 1)
+	if (le64_to_cpu(attr->nres.evcn) == (u64)-1 ||
+	    le64_to_cpu(attr->nres.svcn) > le64_to_cpu(attr->nres.evcn) + 1)
 		goto out;
 
 	data_size = le64_to_cpu(attr->nres.data_size);
-- 
2.43.0

Re: [PATCH] fs/ntfs3: reject evcn == U64_MAX in mi_enum_attr()

[David Laight]

On Fri, 24 Apr 2026 10:46:19 +0800
Zhan Xusheng <zhanxusheng1024@gmail.com> wrote:

> In mi_enum_attr(), the start/end VCN validation for non-resident
> attributes is:
> 
>     if (svcn > evcn + 1) goto out;
> 
> When on-disk evcn is 0xFFFFFFFFFFFFFFFF (U64_MAX), the addition
> evcn + 1 wraps to 0, and the check becomes "if (svcn > 0)" which
> incorrectly accepts svcn == 0 with evcn == U64_MAX.
> 
> mi_enum_attr() is the core attribute iterator used throughout ntfs3.
> Allowing this malformed VCN range to pass the initial validation can
> feed a bogus 64-bit extent span into downstream code that computes
> evcn + 1 - svcn (producing 0 due to wrap) or uses evcn as a loop
> bound.
> 
> A crafted NTFS image can trigger this on mount or file access.
> 
> Fix by explicitly rejecting evcn == U64_MAX before the addition.
> 
> Fixes: 013ff63b6494 ("fs/ntfs3: Add more attributes checks in mi_enum_attr()")
> Signed-off-by: Zhan Xusheng <zhanxusheng@xiaomi.com>
> ---
>  fs/ntfs3/record.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/ntfs3/record.c b/fs/ntfs3/record.c
> index 32bdb034c2a3..2ff28bfbedad 100644
> --- a/fs/ntfs3/record.c
> +++ b/fs/ntfs3/record.c
> @@ -311,7 +311,8 @@ struct ATTRIB *mi_enum_attr(struct ntfs_inode *ni, struct mft_inode *mi,
>  		goto out;
>  
>  	/* Check start/end vcn. */
> -	if (le64_to_cpu(attr->nres.svcn) > le64_to_cpu(attr->nres.evcn) + 1)
> +	if (le64_to_cpu(attr->nres.evcn) == (u64)-1 ||
> +	    le64_to_cpu(attr->nres.svcn) > le64_to_cpu(attr->nres.evcn) + 1)

Isn't that just:
	if (le64_to_cpu(attr->nres.svcn) >= le64_to_cpu(attr->nres.evcn))

	David

>  		goto out;
>  
>  	data_size = le64_to_cpu(attr->nres.data_size);

Re: [PATCH] fs/ntfs3: reject evcn == U64_MAX in mi_enum_attr()

[Zhan Xusheng]

Hi David,

That would reject the svcn == evcn case, which represents a
single-cluster extent (e.g. svcn=0, evcn=0) and is currently
treated as valid.

The original check allows svcn == evcn + 1 (empty range), so the
condition is strictly "greater than", not "greater than or equal".

The issue here is the overflow of (evcn + 1) when evcn == U64_MAX,
which turns the check into "svcn > 0" and incorrectly allows
svcn == 0.

My patch preserves the existing semantics and only adds the
missing U64_MAX guard.

Thanks,
Zhan Xusheng

Re: [PATCH] fs/ntfs3: reject evcn == U64_MAX in mi_enum_attr()

[David Laight]

On Fri, 24 Apr 2026 21:20:31 +0800
Zhan Xusheng <zhanxusheng1024@gmail.com> wrote:

> Hi David,
> 
> That would reject the svcn == evcn case, which represents a
> single-cluster extent (e.g. svcn=0, evcn=0) and is currently
> treated as valid.

Slight brain fade..

> 
> The original check allows svcn == evcn + 1 (empty range), so the
> condition is strictly "greater than", not "greater than or equal".
> 
> The issue here is the overflow of (evcn + 1) when evcn == U64_MAX,
> which turns the check into "svcn > 0" and incorrectly allows
> svcn == 0.

Is that analysis correct? with the current code:
If evcn is 2 then everything except 0, 1, 2 and 3 are errors.
If evcn is -3 then only -1 is an error.
If evcn is -2 no values are errors.
If evcn is -1 then all svcn values except 0 are errors.

Clearly this doesn't make sense if evcn is -1.

But there isn't an obvious reason why svcn == -4, evcn == -1
shouldn't be a valid range.
(There might be a sanity upper limit is evcn for other reasons.)

	David
 
> 
> My patch preserves the existing semantics and only adds the
> missing U64_MAX guard.
> 
> Thanks,
> Zhan Xusheng
> 

Re: [PATCH] fs/ntfs3: reject evcn == U64_MAX in mi_enum_attr()

[Zhan Xusheng]

On Fri, 24 Apr 2026 16:15:58 +0100
David Laight <david.laight.linux@gmail.com> wrote:

> Is that analysis correct? with the current code:
> If evcn is 2 then everything except 0, 1, 2 and 3 are errors.
> If evcn is -3 then only -1 is an error.
> If evcn is -2 no values are errors.
> If evcn is -1 then all svcn values except 0 are errors.
>
> Clearly this doesn't make sense if evcn is -1.
>
> But there isn't an obvious reason why svcn == -4, evcn == -1
> shouldn't be a valid range.
> (There might be a sanity upper limit is evcn for other reasons.)

Thanks for looking into this, David.

The on-disk svcn/evcn fields are __le64 (ntfs.h:339-340), and ntfs3
uses them as u64 throughout -- the internal VCN type CLST is typedef'd
to u32 or u64 (ntfs.h:73-78).  NTFS VCNs are unsigned by definition;
there is no such thing as a negative VCN.

So the signed interpretation doesn't apply here.  In the unsigned
domain, 0xFFFFFFFFFFFFFFFF is simply the maximum u64 value, not -1.
The only issue is the arithmetic wrap: (u64)0xFFFFFFFFFFFFFFFF + 1 == 0.

Regarding a separate upper bound: yes, evcn is also bounded by the
volume's total cluster count, but mi_enum_attr() validates on-disk
MFT records before the superblock fields are necessarily available
to every caller, so the U64_MAX check is the minimal fix for the
overflow.

Thanks,
Zhan Xusheng

Re: [PATCH] fs/ntfs3: reject evcn == U64_MAX in mi_enum_attr()

[David Laight]

On Sat, 25 Apr 2026 10:03:09 +0800
Zhan Xusheng <zhanxusheng1024@gmail.com> wrote:

> On Fri, 24 Apr 2026 16:15:58 +0100
> David Laight <david.laight.linux@gmail.com> wrote:
> 
> > Is that analysis correct? with the current code:
> > If evcn is 2 then everything except 0, 1, 2 and 3 are errors.
> > If evcn is -3 then only -1 is an error.
> > If evcn is -2 no values are errors.
> > If evcn is -1 then all svcn values except 0 are errors.
> >
> > Clearly this doesn't make sense if evcn is -1.
> >
> > But there isn't an obvious reason why svcn == -4, evcn == -1
> > shouldn't be a valid range.
> > (There might be a sanity upper limit is evcn for other reasons.)  
> 
> Thanks for looking into this, David.
> 
> The on-disk svcn/evcn fields are __le64 (ntfs.h:339-340), and ntfs3
> uses them as u64 throughout -- the internal VCN type CLST is typedef'd
> to u32 or u64 (ntfs.h:73-78).  NTFS VCNs are unsigned by definition;
> there is no such thing as a negative VCN.

I was just writing negative values as shorthand for unsigned ones
just below the ~0ull.
Perhaps I should have added the (u64) cast.

> So the signed interpretation doesn't apply here.  In the unsigned
> domain, 0xFFFFFFFFFFFFFFFF is simply the maximum u64 value, not -1.
> The only issue is the arithmetic wrap: (u64)0xFFFFFFFFFFFFFFFF + 1 == 0.
> 
> Regarding a separate upper bound: yes, evcn is also bounded by the
> volume's total cluster count, but mi_enum_attr() validates on-disk
> MFT records before the superblock fields are necessarily available
> to every caller, so the U64_MAX check is the minimal fix for the
> overflow.

True - but your comment is wrong.
And if the code was looping through the range, I suspect there are
other values that would also lead to is looping ~forever.

If the numbers are relative the to physical media then the maximum
size is much smaller.
Basically you can't have 2^64 of anything - there aren't enough atoms.
So you can use a much smaller 'sanity check' - limit possibly earlier on.

	David

> 
> Thanks,
> Zhan Xusheng

[PATCH v2] fs/ntfs3: reject invalid evcn == (u64)-1 in mi_enum_attr()

[Zhan Xusheng]

In mi_enum_attr(), the start/end VCN validation for non-resident
attributes is:
    if (svcn > evcn + 1) goto out;

This relies on evcn + 1 arithmetic, which overflows when evcn
is (u64)-1 and wraps to 0, breaking the intended range check.

In that case, malformed on-disk metadata such as:
    svcn == 0, evcn == (u64)-1
may incorrectly pass validation.

VCN (virtual cluster number) represents cluster indices within
a NTFS volume and is bounded by the physical size of the filesystem.
Therefore, values near or equal to (u64)-1 are not valid on-disk
VCN values and should be treated as malformed metadata.

Reject this case to prevent arithmetic overflow while preserving
the original semantics of allowing svcn <= evcn + 1.

Fixes: 013ff63b6494 ("fs/ntfs3: Add more attributes checks in mi_enum_attr()")
Signed-off-by: Zhan Xusheng <zhanxusheng@xiaomi.com>
---
 fs/ntfs3/record.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/fs/ntfs3/record.c b/fs/ntfs3/record.c
index 32bdb034c2a3..ad752bebb66c 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);
@@ -311,7 +311,10 @@ struct ATTRIB *mi_enum_attr(struct ntfs_inode *ni, struct mft_inode *mi,
 		goto out;
 
 	/* Check start/end vcn. */
-	if (le64_to_cpu(attr->nres.svcn) > le64_to_cpu(attr->nres.evcn) + 1)
+	svcn = le64_to_cpu(attr->nres.svcn);
+	evcn = le64_to_cpu(attr->nres.evcn);
+	/* evcn == (u64)-1 is invalid on-disk VCN */
+	if (evcn == (u64)-1 || svcn > evcn + 1)
 		goto out;
 
 	data_size = le64_to_cpu(attr->nres.data_size);
-- 
2.43.0

[PATCH v3] fs/ntfs3: reject invalid evcn == (u64)-1 in mi_enum_attr()

[Zhan Xusheng]

In mi_enum_attr(), the start/end VCN validation for non-resident
attributes is:
    if (svcn > evcn + 1) goto out;

This relies on evcn + 1 arithmetic, which overflows when evcn
is (u64)-1 and wraps to 0, breaking the intended range check.

In that case, malformed on-disk metadata such as:
    svcn == 0, evcn == (u64)-1
may incorrectly pass validation.

VCN (virtual cluster number) represents cluster indices within
a NTFS volume and is bounded by the physical size of the filesystem.
Therefore, values near or equal to (u64)-1 are not valid on-disk
VCN values and should be treated as malformed metadata.

Reject this case to prevent arithmetic overflow while preserving
the original semantics of allowing svcn <= evcn + 1.

Fixes: 013ff63b6494 ("fs/ntfs3: Add more attributes checks in mi_enum_attr()")
Signed-off-by: Zhan Xusheng <zhanxusheng@xiaomi.com>
---
 fs/ntfs3/record.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/fs/ntfs3/record.c b/fs/ntfs3/record.c
index 32bdb034c2a3..ad752bebb66c 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);
@@ -311,7 +311,10 @@ struct ATTRIB *mi_enum_attr(struct ntfs_inode *ni, struct mft_inode *mi,
 		goto out;
 
 	/* Check start/end vcn. */
-	if (le64_to_cpu(attr->nres.svcn) > le64_to_cpu(attr->nres.evcn) + 1)
+	svcn = le64_to_cpu(attr->nres.svcn);
+	evcn = le64_to_cpu(attr->nres.evcn);
+	/* evcn == (u64)-1 is invalid on-disk VCN */
+	if (evcn == (u64)-1 || svcn > evcn + 1)
 		goto out;
 
 	data_size = le64_to_cpu(attr->nres.data_size);
-- 
2.43.0

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

[fra-build]

From: Zhan Xusheng <zhanxusheng@xiaomi.com>

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; this rejects all out-of-range evcn values,
not just U64_MAX, and as a side effect prevents the "evcn + 1"
wraparound entirely.

Fixes: 013ff63b6494 ("fs/ntfs3: Add more attributes checks in mi_enum_attr()")
Signed-off-by: Zhan Xusheng <zhanxusheng@xiaomi.com>
---
 fs/ntfs3/record.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/fs/ntfs3/record.c b/fs/ntfs3/record.c
index 32bdb034c2a3..4379446bcf96 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,17 @@ 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.  evcn must be a valid cluster index, i.e.
+	 * less than the volume's total cluster count (sbi->used.bitmap.nbits);
+	 * rejecting it here also keeps "svcn > evcn + 1" meaningful, since
+	 * evcn == U64_MAX would wrap "evcn + 1" to 0.  svcn does not need
+	 * its own bound: once evcn < nbits, "svcn > evcn + 1" implies
+	 * svcn <= nbits.
+	 */
+	svcn = le64_to_cpu(attr->nres.svcn);
+	evcn = le64_to_cpu(attr->nres.evcn);
+	if (evcn >= mi->sbi->used.bitmap.nbits || svcn > evcn + 1)
 		goto out;
 
 	data_size = le64_to_cpu(attr->nres.data_size);
-- 
2.43.0

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

[David Laight]

On Fri, 15 May 2026 16:23:05 +0800
fra-build <zhanxusheng1024@gmail.com> wrote:

> From: Zhan Xusheng <zhanxusheng@xiaomi.com>
> 
> 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; this rejects all out-of-range evcn values,
> not just U64_MAX, and as a side effect prevents the "evcn + 1"
> wraparound entirely.
> 
> Fixes: 013ff63b6494 ("fs/ntfs3: Add more attributes checks in mi_enum_attr()")
> Signed-off-by: Zhan Xusheng <zhanxusheng@xiaomi.com>

Reviewed-by: David Laight <david.laight.linux@gmail.com>

> ---
>  fs/ntfs3/record.c | 15 ++++++++++++---
>  1 file changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/ntfs3/record.c b/fs/ntfs3/record.c
> index 32bdb034c2a3..4379446bcf96 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,17 @@ 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.  evcn must be a valid cluster index, i.e.
> +	 * less than the volume's total cluster count (sbi->used.bitmap.nbits);
> +	 * rejecting it here also keeps "svcn > evcn + 1" meaningful, since
> +	 * evcn == U64_MAX would wrap "evcn + 1" to 0.  svcn does not need
> +	 * its own bound: once evcn < nbits, "svcn > evcn + 1" implies
> +	 * svcn <= nbits.
> +	 */
> +	svcn = le64_to_cpu(attr->nres.svcn);
> +	evcn = le64_to_cpu(attr->nres.evcn);
> +	if (evcn >= mi->sbi->used.bitmap.nbits || svcn > evcn + 1)
>  		goto out;
>  
>  	data_size = le64_to_cpu(attr->nres.data_size);

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

[Konstantin Komarov]

On 5/15/26 10:23, fra-build wrote:

> From: Zhan Xusheng <zhanxusheng@xiaomi.com>
>
> 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; this rejects all out-of-range evcn values,
> not just U64_MAX, and as a side effect prevents the "evcn + 1"
> wraparound entirely.
>
> Fixes: 013ff63b6494 ("fs/ntfs3: Add more attributes checks in mi_enum_attr()")
> Signed-off-by: Zhan Xusheng <zhanxusheng@xiaomi.com>
> ---
>   fs/ntfs3/record.c | 15 ++++++++++++---
>   1 file changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/fs/ntfs3/record.c b/fs/ntfs3/record.c
> index 32bdb034c2a3..4379446bcf96 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,17 @@ 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.  evcn must be a valid cluster index, i.e.
> +	 * less than the volume's total cluster count (sbi->used.bitmap.nbits);
> +	 * rejecting it here also keeps "svcn > evcn + 1" meaningful, since
> +	 * evcn == U64_MAX would wrap "evcn + 1" to 0.  svcn does not need
> +	 * its own bound: once evcn < nbits, "svcn > evcn + 1" implies
> +	 * svcn <= nbits.
> +	 */
> +	svcn = le64_to_cpu(attr->nres.svcn);
> +	evcn = le64_to_cpu(attr->nres.evcn);
> +	if (evcn >= mi->sbi->used.bitmap.nbits || svcn > evcn + 1)
>   		goto out;
>   
>   	data_size = le64_to_cpu(attr->nres.data_size);

Hello,

This patch doesn't apply as-is - during internal testing it caused a
number of failures.

I believe the cause is empty non-resident attributes, which are
legitimately encoded with svcn=0 and evcn=U64_MAX (a -1 sentinel for no
allocated clusters). The new evcn >= sbi->used.bitmap.nbits check rejects
those valid attributes. The U64_MAX wraparound you're guarding against is
real, but it needs to be handled without rejecting the empty-attribute
sentinel.

Not applied yet. Thanks.

Regards,
Konstantin

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

[Zhan Xusheng]

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);
-- 
2.43.0