public inbox for ntfs3@lists.linux.dev
 help / color / mirror / Atom feed
* [PATCH] fs: ntfs3: fix infinite loop in attr_load_runs_range on inconsistent metadata
@ 2025-12-02 11:01 Jaehun Gou
  2025-12-09 13:42 ` Konstantin Komarov
  0 siblings, 1 reply; 2+ messages in thread
From: Jaehun Gou @ 2025-12-02 11:01 UTC (permalink / raw)
  To: almaz.alexandrovich
  Cc: ntfs3, linux-kernel, Jaehun Gou, Seunghun Han, Jihoon Kwon

We found an infinite loop bug in the ntfs3 file system that can lead to a
Denial-of-Service (DoS) condition.

A malformed NTFS image can cause an infinite loop when an attribute header
indicates an empty run list, while directory entries reference it as
containing actual data. In NTFS, setting evcn=-1 with svcn=0 is a valid way
to represent an empty run list, and run_unpack() correctly handles this by
checking if evcn + 1 equals svcn and returning early without parsing any run
data. However, this creates a problem when there is metadata inconsistency,
where the attribute header claims to be empty (evcn=-1) but the caller
expects to read actual data. When run_unpack() immediately returns success
upon seeing this condition, it leaves the runs_tree uninitialized with
run->runs as a NULL. The calling function attr_load_runs_range() assumes
that a successful return means that the runs were loaded and sets clen to 0,
expecting the next run_lookup_entry() call to succeed. Because runs_tree
remains uninitialized, run_lookup_entry() continues to fail, and the loop
increments vcn by zero (vcn += 0), leading to an infinite loop.

This patch adds a retry counter to detect when run_lookup_entry() fails
consecutively after attr_load_runs_vcn(). If the run is still not found on
the second attempt, it indicates corrupted metadata and returns -EINVAL,
preventing the Denial-of-Service (DoS) vulnerability.

Co-developed-by: Seunghun Han <kkamagui@gmail.com>
Signed-off-by: Seunghun Han <kkamagui@gmail.com>
Co-developed-by: Jihoon Kwon <kjh010315@gmail.com>
Signed-off-by: Jihoon Kwon <kjh010315@gmail.com>
Signed-off-by: Jaehun Gou <p22gone@gmail.com>
---
 fs/ntfs3/attrib.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/fs/ntfs3/attrib.c b/fs/ntfs3/attrib.c
index eced9013a881..f0ff85b7d76d 100644
--- a/fs/ntfs3/attrib.c
+++ b/fs/ntfs3/attrib.c
@@ -1354,19 +1354,28 @@ int attr_load_runs_range(struct ntfs_inode *ni, enum ATTR_TYPE type,
 	CLST vcn;
 	CLST vcn_last = (to - 1) >> cluster_bits;
 	CLST lcn, clen;
-	int err;
+	int err = 0;
+	int retry = 0;
 
 	for (vcn = from >> cluster_bits; vcn <= vcn_last; vcn += clen) {
 		if (!run_lookup_entry(run, vcn, &lcn, &clen, NULL)) {
+			if (retry != 0) { /* Next run_lookup_entry(vcn) also failed. */
+				err = -EINVAL;
+				break;
+			}
 			err = attr_load_runs_vcn(ni, type, name, name_len, run,
 						 vcn);
 			if (err)
-				return err;
+				break;
+
 			clen = 0; /* Next run_lookup_entry(vcn) must be success. */
+			retry++;
 		}
+		else
+			retry = 0;
 	}
 
-	return 0;
+	return err;
 }
 
 #ifdef CONFIG_NTFS3_LZX_XPRESS
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH] fs: ntfs3: fix infinite loop in attr_load_runs_range on inconsistent metadata
  2025-12-02 11:01 [PATCH] fs: ntfs3: fix infinite loop in attr_load_runs_range on inconsistent metadata Jaehun Gou
@ 2025-12-09 13:42 ` Konstantin Komarov
  0 siblings, 0 replies; 2+ messages in thread
From: Konstantin Komarov @ 2025-12-09 13:42 UTC (permalink / raw)
  To: Jaehun Gou; +Cc: ntfs3, linux-kernel, Seunghun Han, Jihoon Kwon

On 12/2/25 12:01, Jaehun Gou wrote:

> We found an infinite loop bug in the ntfs3 file system that can lead to a
> Denial-of-Service (DoS) condition.
>
> A malformed NTFS image can cause an infinite loop when an attribute header
> indicates an empty run list, while directory entries reference it as
> containing actual data. In NTFS, setting evcn=-1 with svcn=0 is a valid way
> to represent an empty run list, and run_unpack() correctly handles this by
> checking if evcn + 1 equals svcn and returning early without parsing any run
> data. However, this creates a problem when there is metadata inconsistency,
> where the attribute header claims to be empty (evcn=-1) but the caller
> expects to read actual data. When run_unpack() immediately returns success
> upon seeing this condition, it leaves the runs_tree uninitialized with
> run->runs as a NULL. The calling function attr_load_runs_range() assumes
> that a successful return means that the runs were loaded and sets clen to 0,
> expecting the next run_lookup_entry() call to succeed. Because runs_tree
> remains uninitialized, run_lookup_entry() continues to fail, and the loop
> increments vcn by zero (vcn += 0), leading to an infinite loop.
>
> This patch adds a retry counter to detect when run_lookup_entry() fails
> consecutively after attr_load_runs_vcn(). If the run is still not found on
> the second attempt, it indicates corrupted metadata and returns -EINVAL,
> preventing the Denial-of-Service (DoS) vulnerability.
>
> Co-developed-by: Seunghun Han <kkamagui@gmail.com>
> Signed-off-by: Seunghun Han <kkamagui@gmail.com>
> Co-developed-by: Jihoon Kwon <kjh010315@gmail.com>
> Signed-off-by: Jihoon Kwon <kjh010315@gmail.com>
> Signed-off-by: Jaehun Gou <p22gone@gmail.com>
> ---
>   fs/ntfs3/attrib.c | 15 ++++++++++++---
>   1 file changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/fs/ntfs3/attrib.c b/fs/ntfs3/attrib.c
> index eced9013a881..f0ff85b7d76d 100644
> --- a/fs/ntfs3/attrib.c
> +++ b/fs/ntfs3/attrib.c
> @@ -1354,19 +1354,28 @@ int attr_load_runs_range(struct ntfs_inode *ni, enum ATTR_TYPE type,
>   	CLST vcn;
>   	CLST vcn_last = (to - 1) >> cluster_bits;
>   	CLST lcn, clen;
> -	int err;
> +	int err = 0;
> +	int retry = 0;
>   
>   	for (vcn = from >> cluster_bits; vcn <= vcn_last; vcn += clen) {
>   		if (!run_lookup_entry(run, vcn, &lcn, &clen, NULL)) {
> +			if (retry != 0) { /* Next run_lookup_entry(vcn) also failed. */
> +				err = -EINVAL;
> +				break;
> +			}
>   			err = attr_load_runs_vcn(ni, type, name, name_len, run,
>   						 vcn);
>   			if (err)
> -				return err;
> +				break;
> +
>   			clen = 0; /* Next run_lookup_entry(vcn) must be success. */
> +			retry++;
>   		}
> +		else
> +			retry = 0;
>   	}
>   
> -	return 0;
> +	return err;
>   }
>   
>   #ifdef CONFIG_NTFS3_LZX_XPRESS

Applied, will be sent in the next pull request.

Regards,
Konstantin


^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2025-12-09 13:42 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-12-02 11:01 [PATCH] fs: ntfs3: fix infinite loop in attr_load_runs_range on inconsistent metadata Jaehun Gou
2025-12-09 13:42 ` Konstantin Komarov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox