[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