[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <b66104c4-e256-4ed0-88dd-476415c5f0a1@paragon-software.com>
Date: Tue, 9 Dec 2025 14:42:00 +0100
From: Konstantin Komarov <almaz.alexandrovich@...agon-software.com>
To: Jaehun Gou <p22gone@...il.com>
CC: <ntfs3@...ts.linux.dev>, <linux-kernel@...r.kernel.org>, Seunghun Han
<kkamagui@...il.com>, Jihoon Kwon <kjh010315@...il.com>
Subject: Re: [PATCH] fs: ntfs3: fix infinite loop in attr_load_runs_range on
inconsistent metadata
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@...il.com>
> Signed-off-by: Seunghun Han <kkamagui@...il.com>
> Co-developed-by: Jihoon Kwon <kjh010315@...il.com>
> Signed-off-by: Jihoon Kwon <kjh010315@...il.com>
> Signed-off-by: Jaehun Gou <p22gone@...il.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
Powered by blists - more mailing lists