[PATCH] ntfs: validate error codes in check_windows_hibernation_status()

[PATCH] ntfs: validate error codes in check_windows_hibernation_status()

[Hongling Zeng]

check_windows_hibernation_status() calls ntfs_lookup_inode_by_name()
which returns MFT references read directly from disk (untrusted data).
The current code extracts error codes via MREF_ERR() without proper
validation, allowing maliciously crafted NTFS images to trigger
incorrect error handling.

The MFT reference encoding uses bit 47 as an error indicator, but the
lower 32 bits can contain arbitrary values. If a malicious image sets
the error bit with a positive integer (e.g., 1), MREF_ERR() returns
that positive value. This can cause the function to incorrectly
interpret the error as "Windows is hibernated" status, potentially
leading to the filesystem being mounted read-only (denial of service).

Fix by strictly validating error codes: only accept negative values
in the valid errno range [-MAX_ERRNO, -1]. Convert all other values
(positive, zero, or out-of-range) to -EIO to indicate disk corruption.

This prevents potential security issues and ensures proper error handling
for corrupted or malicious NTFS filesystems.

Fixes: 1e9ea7e04472d ("Revert \"fs: Remove NTFS classic\"")
Cc: stable@vger.kernel.org
Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>
---
 fs/ntfs/super.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/fs/ntfs/super.c b/fs/ntfs/super.c
index 8abe7bee4c0d..78f7e9fa76a0 100644
--- a/fs/ntfs/super.c
+++ b/fs/ntfs/super.c
@@ -1168,9 +1168,16 @@ static int check_windows_hibernation_status(struct ntfs_volume *vol)
 			ntfs_debug("hiberfil.sys not present.  Windows is not hibernated on the volume.");
 			return 0;
 		}
-		/* A real error occurred. */
-		ntfs_error(vol->sb, "Failed to find inode number for hiberfil.sys.");
-		return ret;
+		/* Validate error code from untrusted disk data. */
+		if (ret < 0 && ret >= -MAX_ERRNO) {
+			ntfs_error(vol->sb, "Failed to find inode number for hiberfil.sys.");
+			return ret;
+		}
+		/* Invalid error code indicates disk corruption. */
+		ntfs_error(vol->sb,
+			"hiberfil.sys lookup returned invalid error code %i, treating as disk corruption.",
+				ret);
+		return -EIO;
 	}
 	/* Get the inode. */
 	vi = ntfs_iget(vol->sb, MREF(mref));
-- 
2.25.1

Re: [PATCH] ntfs: validate error codes in check_windows_hibernation_status()

[Namjae Jeon]

On Thu, Jul 2, 2026 at 12:37 PM Hongling Zeng <zenghongling@kylinos.cn> wrote:
>
> check_windows_hibernation_status() calls ntfs_lookup_inode_by_name()
> which returns MFT references read directly from disk (untrusted data).
> The current code extracts error codes via MREF_ERR() without proper
> validation, allowing maliciously crafted NTFS images to trigger
> incorrect error handling.
>
> The MFT reference encoding uses bit 47 as an error indicator, but the
> lower 32 bits can contain arbitrary values. If a malicious image sets
> the error bit with a positive integer (e.g., 1), MREF_ERR() returns
> that positive value. This can cause the function to incorrectly
> interpret the error as "Windows is hibernated" status, potentially
> leading to the filesystem being mounted read-only (denial of service).
>
> Fix by strictly validating error codes: only accept negative values
> in the valid errno range [-MAX_ERRNO, -1]. Convert all other values
> (positive, zero, or out-of-range) to -EIO to indicate disk corruption.
>
> This prevents potential security issues and ensures proper error handling
> for corrupted or malicious NTFS filesystems.
>
> Fixes: 1e9ea7e04472d ("Revert \"fs: Remove NTFS classic\"")
> Cc: stable@vger.kernel.org
> Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>
I think this should be fixed in ntfs_lookup_inode_by_name(), rather
than in the caller.
And I will revert your previous patch ("ntfs: validate error codes
from untrusted disk data").

Thanks.

Re: [PATCH] ntfs: validate error codes in check_windows_hibernation_status()

[Hongling Zeng]

Looking at ntfs_lookup_inode_by_name() more carefully:

All error return paths inside the function use hardcoded kernel errnos
(MREF_ERR(-ENOENT), MREF_ERR(-EIO), MREF_ERR(-ENOMEM)) - these are
already valid by construction.

The actual risk occurs when the function returns a "successful" MFT
reference from disk (ie->data.dir.indexed_file) that happens to have
bit 47 set - making IS_ERR_MREF() true at the caller. In this case,
MREF_ERR() extracts garbage from untrusted disk data.

This cannot be fixed inside ntfs_lookup_inode_by_name() without
changing its return value semantics, because from the function's
perspective it found a matching index entry and returned it. Only
the caller, after IS_ERR_MREF() triggers, is in a position to
validate that the extracted error code is a legitimate errno.

Restructuring the function to distinguish "real errors I generated"
from "disk data that looks like an error" would require a more
invasive API change (e.g., returning int + out-parameter), which
seems inappropriate for a legacy filesystem in maintenance mode.

在 2026年07月03日 15:06, Namjae Jeon 写道:
> On Thu, Jul 2, 2026 at 12:37 PM Hongling Zeng <zenghongling@kylinos.cn> wrote:
>> check_windows_hibernation_status() calls ntfs_lookup_inode_by_name()
>> which returns MFT references read directly from disk (untrusted data).
>> The current code extracts error codes via MREF_ERR() without proper
>> validation, allowing maliciously crafted NTFS images to trigger
>> incorrect error handling.
>>
>> The MFT reference encoding uses bit 47 as an error indicator, but the
>> lower 32 bits can contain arbitrary values. If a malicious image sets
>> the error bit with a positive integer (e.g., 1), MREF_ERR() returns
>> that positive value. This can cause the function to incorrectly
>> interpret the error as "Windows is hibernated" status, potentially
>> leading to the filesystem being mounted read-only (denial of service).
>>
>> Fix by strictly validating error codes: only accept negative values
>> in the valid errno range [-MAX_ERRNO, -1]. Convert all other values
>> (positive, zero, or out-of-range) to -EIO to indicate disk corruption.
>>
>> This prevents potential security issues and ensures proper error handling
>> for corrupted or malicious NTFS filesystems.
>>
>> Fixes: 1e9ea7e04472d ("Revert \"fs: Remove NTFS classic\"")
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>
> I think this should be fixed in ntfs_lookup_inode_by_name(), rather
> than in the caller.
> And I will revert your previous patch ("ntfs: validate error codes
> from untrusted disk data").
>
> Thanks.

Re: [PATCH] ntfs: validate error codes in check_windows_hibernation_status()

[Namjae Jeon]

On Fri, Jul 3, 2026 at 5:25 PM Hongling Zeng <zhongling0719@126.com> wrote:
>
> Looking at ntfs_lookup_inode_by_name() more carefully:
>
> All error return paths inside the function use hardcoded kernel errnos
> (MREF_ERR(-ENOENT), MREF_ERR(-EIO), MREF_ERR(-ENOMEM)) - these are
> already valid by construction.
>
> The actual risk occurs when the function returns a "successful" MFT
> reference from disk (ie->data.dir.indexed_file) that happens to have
> bit 47 set - making IS_ERR_MREF() true at the caller. In this case,
> MREF_ERR() extracts garbage from untrusted disk data.
>
> This cannot be fixed inside ntfs_lookup_inode_by_name() without
> changing its return value semantics, because from the function's
> perspective it found a matching index entry and returned it. Only
> the caller, after IS_ERR_MREF() triggers, is in a position to
> validate that the extracted error code is a legitimate errno.
>
> Restructuring the function to distinguish "real errors I generated"
> from "disk data that looks like an error" would require a more
> invasive API change (e.g., returning int + out-parameter), which
> seems inappropriate for a legacy filesystem in maintenance mode.

I think it would be good to fix it like this. Let me know your opinion.

diff --git a/fs/ntfs/dir.c b/fs/ntfs/dir.c
index 4b6bd5f30c65..6fa9ae3377cb 100644
--- a/fs/ntfs/dir.c
+++ b/fs/ntfs/dir.c
@@ -23,6 +23,13 @@
 __le16 I30[5] = { cpu_to_le16('$'), cpu_to_le16('I'),
                cpu_to_le16('3'),       cpu_to_le16('0'), 0 };

+static inline u64 ntfs_check_mref(u64 mref)
+{
+       if (IS_ERR_MREF(mref))
+               return ERR_MREF(-EIO);
+       return mref;
+}
+
 /*
  * ntfs_lookup_inode_by_name - find an inode in a directory given its name
  * @dir_ni:    ntfs inode of the directory in which to search for the name
@@ -178,7 +185,7 @@ u64 ntfs_lookup_inode_by_name(struct ntfs_inode
*dir_ni, const __le16 *uname,
                        mref = le64_to_cpu(ie->data.dir.indexed_file);
                        ntfs_attr_put_search_ctx(ctx);
                        unmap_mft_record(dir_ni);
-                       return mref;
+                       return ntfs_check_mref(mref);
                }
                /*
                 * For a case insensitive mount, we also perform a case
@@ -273,7 +280,7 @@ u64 ntfs_lookup_inode_by_name(struct ntfs_inode
*dir_ni, const __le16 *uname,
                if (name) {
                        ntfs_attr_put_search_ctx(ctx);
                        unmap_mft_record(dir_ni);
-                       return name->mref;
+                       return ntfs_check_mref(name->mref);
                }
                ntfs_debug("Entry not found.");
                err = -ENOENT;
@@ -413,7 +420,7 @@ u64 ntfs_lookup_inode_by_name(struct ntfs_inode
*dir_ni, const __le16 *uname,
                        mref = le64_to_cpu(ie->data.dir.indexed_file);
                        kfree(kaddr);
                        iput(ia_vi);
-                       return mref;
+                       return ntfs_check_mref(mref);
                }
                /*
                 * For a case insensitive mount, we also perform a case
@@ -538,7 +545,7 @@ u64 ntfs_lookup_inode_by_name(struct ntfs_inode
*dir_ni, const __le16 *uname,
        if (name) {
                kfree(kaddr);
                iput(ia_vi);
-               return name->mref;
+               return ntfs_check_mref(name->mref);
        }
        ntfs_debug("Entry not found.");
        err = -ENOENT;