[PATCH 0/2] ntfs: fix link name free and $FILE_NAME validation

[PATCH 0/2] ntfs: fix link name free and $FILE_NAME validation

[DaeMyung Kang]

This series contains two independent fixes for fs/ntfs.

Patch 1 corrects ntfs_link()'s release of the new link name.
ntfs_nlstoucs() returns a buffer allocated from ntfs_name_cache, and
every other namei.c caller releases it with kmem_cache_free(); only
ntfs_link() used kfree(). The conversion failure path is also adjusted
so the common out label is not reached with a NULL pointer that would
hit kmem_cache_free().

Patch 2 validates that a resident $FILE_NAME attribute can actually
hold the variable-length UTF-16 name implied by file_name_length.
A crafted image was reproduced under KASAN as a slab-out-of-bounds
read from a kmalloc-1k MFT record copy through ntfs_attr_name_get().
The validation is added before ntfs_attr_find() and
ntfs_external_attr_find() return the attribute, covering both the
AT_FILE_NAME and AT_UNUSED enumeration lookup paths.

The two patches are independent and can be applied in either order.

DaeMyung Kang (2):
  ntfs: free link name from ntfs_name_cache
  ntfs: validate resident file name attribute length

 fs/ntfs/attrib.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
 fs/ntfs/namei.c  |  5 ++---
 2 files changed, 48 insertions(+), 3 deletions(-)

-- 
2.43.0

[PATCH 1/2] ntfs: free link name from ntfs_name_cache

[DaeMyung Kang]

ntfs_link() converts the new link name with ntfs_nlstoucs() using
NTFS_MAX_NAME_LEN. In this case ntfs_nlstoucs() allocates the result
from ntfs_name_cache, and its contract requires callers to release the
buffer with kmem_cache_free(ntfs_name_cache, ...).

All other ntfs_nlstoucs() callers in namei.c do that, but ntfs_link()
uses kfree(), which mismatches the allocator for successfully converted
names.

The conversion failure path reaches the common out label with uname ==
NULL. That was harmless for kfree(), but kmem_cache_free() does not
provide the same NULL contract. Return directly on conversion failure
and free successful conversions with ntfs_name_cache.

Fixes: af0db57d4293 ("ntfs: update inode operations")
Signed-off-by: DaeMyung Kang <charsyam@gmail.com>
---
 fs/ntfs/namei.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/fs/ntfs/namei.c b/fs/ntfs/namei.c
index c4f82846c58c..9c1c36acfad2 100644
--- a/fs/ntfs/namei.c
+++ b/fs/ntfs/namei.c
@@ -1532,8 +1532,7 @@ static int ntfs_link(struct dentry *old_dentry, struct inode *dir,
 	if (uname_len < 0) {
 		if (uname_len != -ENAMETOOLONG)
 			ntfs_error(sb, "Failed to convert name to unicode.");
-		err = -ENOMEM;
-		goto out;
+		return -ENOMEM;
 	}
 
 	if (!(vol->vol_flags & VOLUME_IS_DIRTY))
@@ -1563,7 +1562,7 @@ static int ntfs_link(struct dentry *old_dentry, struct inode *dir,
 	mutex_unlock(&ni->mrec_lock);
 
 out:
-	kfree(uname);
+	kmem_cache_free(ntfs_name_cache, uname);
 	return err;
 }
 
-- 
2.43.0

Re: [PATCH 1/2] ntfs: free link name from ntfs_name_cache

[Namjae Jeon]

On Sun, May 24, 2026 at 2:42 PM DaeMyung Kang <charsyam@gmail.com> wrote:
>
> ntfs_link() converts the new link name with ntfs_nlstoucs() using
> NTFS_MAX_NAME_LEN. In this case ntfs_nlstoucs() allocates the result
> from ntfs_name_cache, and its contract requires callers to release the
> buffer with kmem_cache_free(ntfs_name_cache, ...).
>
> All other ntfs_nlstoucs() callers in namei.c do that, but ntfs_link()
> uses kfree(), which mismatches the allocator for successfully converted
> names.
>
> The conversion failure path reaches the common out label with uname ==
> NULL. That was harmless for kfree(), but kmem_cache_free() does not
> provide the same NULL contract. Return directly on conversion failure
> and free successful conversions with ntfs_name_cache.
>
> Fixes: af0db57d4293 ("ntfs: update inode operations")
> Signed-off-by: DaeMyung Kang <charsyam@gmail.com>
Applied it to #ntfs-next.
Thanks!

[PATCH 2/2] ntfs: validate resident file name attribute length

[DaeMyung Kang]

ntfs_attr_find() and ntfs_external_attr_find() check that generic
resident attribute values fit in their attribute records and that
fixed-size resident values are large enough. For $FILE_NAME, however,
the fixed part is not enough: the value also contains a variable-length
UTF-16 name whose length is stored in file_name_length.

A crafted image can set a small resident value_length while leaving
file_name_length large. Callers then trust file_name_length and read
past the resident value when converting or comparing the name. This was
reproduced with a crafted image under KASAN as a slab-out-of-bounds read
from the kmalloc-1k MFT record copy, for example through:

  ntfs_lookup()
  ntfs_iget()
  ntfs_read_locked_inode()
  ntfs_attr_name_get()
  ntfs_ucstonls()
  utf16s_to_utf8s()

Validate $FILE_NAME before lookup can return the current attribute,
including the AT_UNUSED enumeration case where callers inspect returned
attributes directly. Reuse the same self-contained helper for both the
base-record lookup path and the attribute-list lookup path. Log a
specific corruption message at the rejection point, matching the
existing attribute-name validation in ntfs_attr_find().

Reject non-resident $FILE_NAME records too: the format requires
$FILE_NAME to be resident and callers treat returned records as
resident.

Fixes: 6ceb4cc81ef3 ("ntfs: add bound checking to ntfs_attr_find")
Signed-off-by: DaeMyung Kang <charsyam@gmail.com>
---
 fs/ntfs/attrib.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 46 insertions(+)

diff --git a/fs/ntfs/attrib.c b/fs/ntfs/attrib.c
index 421c6cdcbb53..5643a097eb11 100644
--- a/fs/ntfs/attrib.c
+++ b/fs/ntfs/attrib.c
@@ -595,6 +595,37 @@ static u32 ntfs_resident_attr_min_value_length(const __le32 type)
 	}
 }
 
+static bool ntfs_file_name_attr_is_valid(const struct attr_record *a)
+{
+	const struct file_name_attr *fn;
+	u32 attr_len = le32_to_cpu(a->length);
+	u32 value_length;
+	u16 value_offset;
+	u32 file_name_size;
+
+	if (a->non_resident)
+		return false;
+
+	if (attr_len < offsetof(struct attr_record, data.resident.reserved) +
+			sizeof(a->data.resident.reserved))
+		return false;
+
+	value_length = le32_to_cpu(a->data.resident.value_length);
+	value_offset = le16_to_cpu(a->data.resident.value_offset);
+
+	if (value_length > attr_len || value_offset > attr_len - value_length)
+		return false;
+
+	if (value_length < ntfs_resident_attr_min_value_length(AT_FILE_NAME))
+		return false;
+
+	fn = (const struct file_name_attr *)((const u8 *)a + value_offset);
+	file_name_size = fn->file_name_length * sizeof(__le16);
+
+	return file_name_size <=
+			value_length - offsetof(struct file_name_attr, file_name);
+}
+
 /*
  * ntfs_attr_find - find (next) attribute in mft record
  * @type:	attribute type to find
@@ -705,6 +736,13 @@ static int ntfs_attr_find(const __le32 type, const __le16 *name,
 			}
 		}
 
+		if (a->type == AT_FILE_NAME &&
+		    !ntfs_file_name_attr_is_valid(a)) {
+			ntfs_error(vol->sb,
+				   "Corrupt $FILE_NAME attribute in MFT record %llu\n",
+				   ctx->ntfs_ino->mft_no);
+			break;
+		}
 		if (type == AT_UNUSED)
 			return 0;
 		if (a->type != type)
@@ -1252,6 +1290,14 @@ static int ntfs_external_attr_find(const __le32 type,
 
 		ctx->attr = a;
 
+		if (a->type == AT_FILE_NAME &&
+		    !ntfs_file_name_attr_is_valid(a)) {
+			ntfs_error(vol->sb,
+				   "Corrupt $FILE_NAME attribute in MFT record %llu\n",
+				   ctx->ntfs_ino->mft_no);
+			break;
+		}
+
 		if (a->non_resident) {
 			u32 min_len;
 			u16 mp_offset;
-- 
2.43.0

Re: [PATCH 2/2] ntfs: validate resident file name attribute length

[Namjae Jeon]

> Validate $FILE_NAME before lookup can return the current attribute,
> including the AT_UNUSED enumeration case where callers inspect returned
> attributes directly. Reuse the same self-contained helper for both the
> base-record lookup path and the attribute-list lookup path. Log a
> specific corruption message at the rejection point, matching the
> existing attribute-name validation in ntfs_attr_find().
>
> Reject non-resident $FILE_NAME records too: the format requires
> $FILE_NAME to be resident and callers treat returned records as
> resident.
Could you refactor the existing min_value_length check into a unified
validation helper that covers all resident attributes ? Other resident
attributes(volume name, index root, ea_information, etc.) with
variable-length could be validated similarly.
Thanks!

[PATCH v2] ntfs: validate resident attribute values on lookup

[DaeMyung Kang]

ntfs_attr_find() and ntfs_external_attr_find() check that generic
resident attribute values fit in their attribute records and that
fixed-size resident values are large enough. For variable-length resident
formats, however, the fixed part is not enough: embedded length fields
can still point callers past the resident value.

A crafted image can set a small resident $FILE_NAME value_length while
leaving file_name_length large. Callers then trust file_name_length and
read past the resident value when converting or comparing the name. This
was reproduced with a crafted image under KASAN as a slab-out-of-bounds
read from the kmalloc-1k MFT record copy. The stack included
ntfs_lookup(), ntfs_iget(), ntfs_read_locked_inode(), ntfs_attr_name_get(),
ntfs_ucstonls(), and utf16s_to_utf8s().

Add a shared resident attribute value validator and use it before a
lookup path can return an attribute, including the AT_UNUSED enumeration
case where callers inspect returned attributes directly. The helper
validates resident value bounds, minimum value sizes, and variable-length
$FILE_NAME and $INDEX_ROOT fields, while preserving the fixed-size
checks for attributes such as $VOLUME_INFORMATION and $EA_INFORMATION.
Log corruption at the rejection point with the attribute name where
known.

Reject non-resident $FILE_NAME records too: the format requires
$FILE_NAME to be resident and callers treat returned records as
resident.

Type-specific format validation for $VOLUME_NAME is intentionally not
added to this helper, even though $VOLUME_NAME is a variable-length
resident attribute. Existing callers such as ntfs_write_volume_label()
treat a failed AT_VOLUME_NAME lookup as an absent label and
unconditionally add a new record afterwards. If the helper rejected
odd-length labels at lookup time, the corrupt record would stay in
place and the caller would append a new record next to it, producing
two $VOLUME_NAME attributes on disk. Caller-side cleanup must precede
any lookup-time rejection of malformed $VOLUME_NAME payloads, so the
type-specific check is left for a follow-up patch. The generic resident
bounds checks (value_offset / value_length / attr_len) still apply to
$VOLUME_NAME, and its entry in the type-name table is kept so that
those failures log with the attribute name.

Fixes: 6ceb4cc81ef3 ("ntfs: add bound checking to ntfs_attr_find")
Signed-off-by: DaeMyung Kang <charsyam@gmail.com>
---
Patch 1/2 of the previous series ("ntfs: free link name from
ntfs_name_cache") has been applied to ntfs-next; this v2 contains only
the remaining patch, with the resident attribute validation reworked per
review feedback.

Changes since v1:
- Refactor resident attribute validation into a shared helper
  ntfs_attr_value_is_valid() used by both ntfs_attr_find() and
  ntfs_external_attr_find(), and remove the duplicated inline check
  from ntfs_external_attr_find().
- Split out resident header/value bounds extraction into
  ntfs_resident_attr_value_get(), returning a small view struct so the
  top-level orchestration reads as: get view -> common min check ->
  per-type format check.
- Validate before the AT_UNUSED enumeration path in ntfs_attr_find()
  can return the attribute to callers that inspect it directly.
- Reject non-resident $FILE_NAME records: the format requires
  $FILE_NAME to be resident and callers treat returned records as
  resident.
- Extend type-specific validation beyond $FILE_NAME to $INDEX_ROOT
  (entries_offset / index_length / allocated_size bounds and 8-byte
  alignment); add $INDEX_ROOT to ntfs_resident_attr_min_value_length().
- Type-specific format validation for $VOLUME_NAME is intentionally
  not added; see the commit message for the caller-side reason.
  Generic resident bounds checks still apply, and the $VOLUME_NAME
  entry in the type-name table is kept so that those failures log
  with a readable attribute name.
- Log corruption at the rejection point with the attribute type name
  via a new ntfs_attr_type_name() helper, matching the existing
  attribute-name corruption message in ntfs_attr_find().

 fs/ntfs/attrib.c | 182 +++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 151 insertions(+), 31 deletions(-)

diff --git a/fs/ntfs/attrib.c b/fs/ntfs/attrib.c
index 421c6cdcbb53..63b3b0d13c45 100644
--- a/fs/ntfs/attrib.c
+++ b/fs/ntfs/attrib.c
@@ -588,6 +588,8 @@ static u32 ntfs_resident_attr_min_value_length(const __le32 type)
 			sizeof(__le16) * 1;
 	case AT_VOLUME_INFORMATION:
 		return sizeof(struct volume_information);
+	case AT_INDEX_ROOT:
+		return sizeof(struct index_root);
 	case AT_EA_INFORMATION:
 		return sizeof(struct ea_information);
 	default:
@@ -595,6 +597,144 @@ static u32 ntfs_resident_attr_min_value_length(const __le32 type)
 	}
 }
 
+static const char *ntfs_attr_type_name(const __le32 type)
+{
+	switch (type) {
+	case AT_STANDARD_INFORMATION:
+		return "$STANDARD_INFORMATION";
+	case AT_FILE_NAME:
+		return "$FILE_NAME";
+	case AT_VOLUME_NAME:
+		return "$VOLUME_NAME";
+	case AT_VOLUME_INFORMATION:
+		return "$VOLUME_INFORMATION";
+	case AT_INDEX_ROOT:
+		return "$INDEX_ROOT";
+	case AT_EA_INFORMATION:
+		return "$EA_INFORMATION";
+	default:
+		return NULL;
+	}
+}
+
+static bool ntfs_file_name_attr_value_is_valid(const u8 *value, const u32 value_length)
+{
+	const struct file_name_attr *fn;
+	u32 file_name_size;
+
+	if (value_length < ntfs_resident_attr_min_value_length(AT_FILE_NAME))
+		return false;
+
+	fn = (const struct file_name_attr *)value;
+	file_name_size = fn->file_name_length * sizeof(__le16);
+
+	return file_name_size <=
+			value_length - offsetof(struct file_name_attr, file_name);
+}
+
+static bool ntfs_index_root_attr_value_is_valid(const u8 *value, const u32 value_length)
+{
+	const struct index_root *ir;
+	u32 index_size;
+	u32 entries_offset;
+	u32 index_length;
+	u32 allocated_size;
+
+	if (value_length < ntfs_resident_attr_min_value_length(AT_INDEX_ROOT))
+		return false;
+
+	ir = (const struct index_root *)value;
+	index_size = value_length - offsetof(struct index_root, index);
+	entries_offset = le32_to_cpu(ir->index.entries_offset);
+	index_length = le32_to_cpu(ir->index.index_length);
+	allocated_size = le32_to_cpu(ir->index.allocated_size);
+
+	if ((entries_offset | index_length | allocated_size) & 7 ||
+	    entries_offset < sizeof(struct index_header) ||
+	    entries_offset > index_length ||
+	    index_length > allocated_size ||
+	    allocated_size > index_size ||
+	    index_length - entries_offset < sizeof(struct index_entry_header))
+		return false;
+
+	return true;
+}
+
+struct ntfs_resident_attr_value {
+	const u8 *data;
+	u32 len;
+};
+
+static bool ntfs_resident_attr_value_get(const struct attr_record *a,
+					 struct ntfs_resident_attr_value *value)
+{
+	u32 attr_len;
+	u16 value_offset;
+
+	attr_len = le32_to_cpu(a->length);
+	if (attr_len < offsetof(struct attr_record, data.resident.reserved) +
+			sizeof(a->data.resident.reserved))
+		return false;
+
+	value->len = le32_to_cpu(a->data.resident.value_length);
+	value_offset = le16_to_cpu(a->data.resident.value_offset);
+
+	if (value->len > attr_len || value_offset > attr_len - value->len)
+		return false;
+
+	value->data = (const u8 *)a + value_offset;
+	return true;
+}
+
+static bool ntfs_attr_value_is_valid(struct ntfs_volume *vol,
+				     const struct attr_record *a,
+				     const u64 mft_no)
+{
+	struct ntfs_resident_attr_value value;
+	const char *type_name;
+	u32 min_len;
+
+	if (a->non_resident) {
+		if (a->type != AT_FILE_NAME)
+			return true;
+		ntfs_error(vol->sb,
+			   "Corrupt non-resident $FILE_NAME attribute in MFT record %llu\n",
+			   mft_no);
+		return false;
+	}
+
+	if (!ntfs_resident_attr_value_get(a, &value))
+		goto corrupt;
+
+	min_len = ntfs_resident_attr_min_value_length(a->type);
+	if (min_len && value.len < min_len)
+		goto corrupt;
+
+	switch (a->type) {
+	case AT_FILE_NAME:
+		if (!ntfs_file_name_attr_value_is_valid(value.data, value.len))
+			goto corrupt;
+		break;
+	case AT_INDEX_ROOT:
+		if (!ntfs_index_root_attr_value_is_valid(value.data, value.len))
+			goto corrupt;
+		break;
+	}
+	return true;
+
+corrupt:
+	type_name = ntfs_attr_type_name(a->type);
+	if (type_name)
+		ntfs_error(vol->sb,
+			   "Corrupt resident %s attribute in MFT record %llu\n",
+			   type_name, mft_no);
+	else
+		ntfs_error(vol->sb,
+			   "Corrupt resident %#x attribute in MFT record %llu\n",
+			   le32_to_cpu(a->type), mft_no);
+	return false;
+}
+
 /*
  * ntfs_attr_find - find (next) attribute in mft record
  * @type:	attribute type to find
@@ -705,8 +845,11 @@ static int ntfs_attr_find(const __le32 type, const __le16 *name,
 			}
 		}
 
-		if (type == AT_UNUSED)
+		if (type == AT_UNUSED) {
+			if (!ntfs_attr_value_is_valid(vol, a, ctx->ntfs_ino->mft_no))
+				break;
 			return 0;
+		}
 		if (a->type != type)
 			continue;
 		/*
@@ -747,24 +890,11 @@ static int ntfs_attr_find(const __le32 type, const __le16 *name,
 			}
 		}
 
-		 /* Validate attribute's value offset/length */
-		if (!a->non_resident) {
-			u32 min_len;
-			u32 value_length = le32_to_cpu(a->data.resident.value_length);
-			u16 value_offset = le16_to_cpu(a->data.resident.value_offset);
-
-			if (value_length > le32_to_cpu(a->length) ||
-			    value_offset > le32_to_cpu(a->length) - value_length)
-				break;
+		if (!ntfs_attr_value_is_valid(vol, a, ctx->ntfs_ino->mft_no))
+			break;
 
-			min_len = ntfs_resident_attr_min_value_length(a->type);
-			if (min_len && value_length < min_len) {
-				ntfs_error(vol->sb,
-					   "Too small %#x resident attribute value in MFT record %lld\n",
-					   le32_to_cpu(a->type), (long long)ctx->ntfs_ino->mft_no);
-				break;
-			}
-		} else {
+		/* Validate non-resident mapping-pairs fields. */
+		if (a->non_resident) {
 			u32 min_len;
 			u16 mp_offset;
 
@@ -1252,6 +1382,9 @@ static int ntfs_external_attr_find(const __le32 type,
 
 		ctx->attr = a;
 
+		if (!ntfs_attr_value_is_valid(vol, a, ctx->ntfs_ino->mft_no))
+			break;
+
 		if (a->non_resident) {
 			u32 min_len;
 			u16 mp_offset;
@@ -1279,19 +1412,6 @@ static int ntfs_external_attr_find(const __le32 type,
 			u32 value_length = le32_to_cpu(a->data.resident.value_length);
 			u16 value_offset = le16_to_cpu(a->data.resident.value_offset);
 
-			if (attr_len < offsetof(struct attr_record, data.resident.reserved) +
-					sizeof(a->data.resident.reserved))
-				break;
-			if (value_length > attr_len || value_offset > attr_len - value_length)
-				break;
-
-			value_length = ntfs_resident_attr_min_value_length(a->type);
-			if (value_length && le32_to_cpu(a->data.resident.value_length) <
-			    value_length) {
-				pr_err("Too small resident attribute value in MFT record %lld, type %#x\n",
-				       (long long)ctx->ntfs_ino->mft_no, a->type);
-				break;
-			}
 			if (value_length == val_len &&
 			    !memcmp((u8 *)a + value_offset, val, val_len)) {
 attr_found:
-- 
2.43.0

Re: [PATCH v2] ntfs: validate resident attribute values on lookup

[Namjae Jeon]

On Tue, May 26, 2026 at 1:33 AM DaeMyung Kang <charsyam@gmail.com> wrote:
>
> ntfs_attr_find() and ntfs_external_attr_find() check that generic
> resident attribute values fit in their attribute records and that
> fixed-size resident values are large enough. For variable-length resident
> formats, however, the fixed part is not enough: embedded length fields
> can still point callers past the resident value.
>
> A crafted image can set a small resident $FILE_NAME value_length while
> leaving file_name_length large. Callers then trust file_name_length and
> read past the resident value when converting or comparing the name. This
> was reproduced with a crafted image under KASAN as a slab-out-of-bounds
> read from the kmalloc-1k MFT record copy. The stack included
> ntfs_lookup(), ntfs_iget(), ntfs_read_locked_inode(), ntfs_attr_name_get(),
> ntfs_ucstonls(), and utf16s_to_utf8s().
>
> Add a shared resident attribute value validator and use it before a
> lookup path can return an attribute, including the AT_UNUSED enumeration
> case where callers inspect returned attributes directly. The helper
> validates resident value bounds, minimum value sizes, and variable-length
> $FILE_NAME and $INDEX_ROOT fields, while preserving the fixed-size
> checks for attributes such as $VOLUME_INFORMATION and $EA_INFORMATION.
> Log corruption at the rejection point with the attribute name where
> known.
>
> Reject non-resident $FILE_NAME records too: the format requires
> $FILE_NAME to be resident and callers treat returned records as
> resident.
>
> Type-specific format validation for $VOLUME_NAME is intentionally not
> added to this helper, even though $VOLUME_NAME is a variable-length
> resident attribute. Existing callers such as ntfs_write_volume_label()
> treat a failed AT_VOLUME_NAME lookup as an absent label and
> unconditionally add a new record afterwards. If the helper rejected
> odd-length labels at lookup time, the corrupt record would stay in
> place and the caller would append a new record next to it, producing
> two $VOLUME_NAME attributes on disk. Caller-side cleanup must precede
> any lookup-time rejection of malformed $VOLUME_NAME payloads, so the
> type-specific check is left for a follow-up patch. The generic resident
> bounds checks (value_offset / value_length / attr_len) still apply to
> $VOLUME_NAME, and its entry in the type-name table is kept so that
> those failures log with the attribute name.
>
> Fixes: 6ceb4cc81ef3 ("ntfs: add bound checking to ntfs_attr_find")
> Signed-off-by: DaeMyung Kang <charsyam@gmail.com>
> ---
> Patch 1/2 of the previous series ("ntfs: free link name from
> ntfs_name_cache") has been applied to ntfs-next; this v2 contains only
> the remaining patch, with the resident attribute validation reworked per
> review feedback.
>
> Changes since v1:
> - Refactor resident attribute validation into a shared helper
>   ntfs_attr_value_is_valid() used by both ntfs_attr_find() and
>   ntfs_external_attr_find(), and remove the duplicated inline check
>   from ntfs_external_attr_find().
> - Split out resident header/value bounds extraction into
>   ntfs_resident_attr_value_get(), returning a small view struct so the
>   top-level orchestration reads as: get view -> common min check ->
>   per-type format check.
> - Validate before the AT_UNUSED enumeration path in ntfs_attr_find()
>   can return the attribute to callers that inspect it directly.
> - Reject non-resident $FILE_NAME records: the format requires
>   $FILE_NAME to be resident and callers treat returned records as
>   resident.
> - Extend type-specific validation beyond $FILE_NAME to $INDEX_ROOT
>   (entries_offset / index_length / allocated_size bounds and 8-byte
>   alignment); add $INDEX_ROOT to ntfs_resident_attr_min_value_length().
> - Type-specific format validation for $VOLUME_NAME is intentionally
>   not added; see the commit message for the caller-side reason.
>   Generic resident bounds checks still apply, and the $VOLUME_NAME
>   entry in the type-name table is kept so that those failures log
>   with a readable attribute name.
> - Log corruption at the rejection point with the attribute type name
>   via a new ntfs_attr_type_name() helper, matching the existing
>   attribute-name corruption message in ntfs_attr_find().
Please check xfstests generic/001 test
failure.(https://github.com/namjaejeon/linux-ntfs/actions/runs/26424312505/job/77785594232)

FSTYP -- ntfs
570PLATFORM -- Linux/x86_64 runnervmg397c 6.17.0-1013-azure
#13~24.04.1-Ubuntu SMP Wed Apr 15 16:52:17 UTC 2026
571MKFS_OPTIONS -- -q --partition-start=0 --sectors-per-track=0
--heads=0 /dev/loop21
572MOUNT_OPTIONS -- /dev/loop21 /mnt/scratch
573
574generic/001 _check_generic_filesystem: filesystem on /dev/loop20 is
inconsistent
575(see /home/runner/work/linux-ntfs/linux-ntfs/exfat-testsuites/xfstests-exfat/results//generic/001.full
for details)
576Trying to repair broken TEST_DEV file system
577- output mismatch (see
/home/runner/work/linux-ntfs/linux-ntfs/exfat-testsuites/xfstests-exfat/results//generic/001.out.bad)
578 --- tests/generic/001.out 2025-01-14 04:54:54.000000000 +0000
579 +++ /home/runner/work/linux-ntfs/linux-ntfs/exfat-testsuites/xfstests-exfat/results//generic/001.out.bad
2026-05-26 00:01:44.663029196 +0000
580 @@ -7,3 +7,4 @@
581 iter 4 chain ... check ....................................
582 iter 5 chain ... check ....................................
583 cleanup
584 +rm: cannot remove '/mnt/test/83726/sub': Input/output error
585 ...
586 (Run 'diff -u
/home/runner/work/linux-ntfs/linux-ntfs/exfat-testsuites/xfstests-exfat/tests/generic/001.out
/home/runner/work/linux-ntfs/linux-ntfs/exfat-testsuites/xfstests-exfat/results//generic/001.out.bad'
to see the entire diff)
587Ran: generic/001
588Failures: generic/001
589Failed 1 of 1 tests

Re: [PATCH v2] ntfs: validate resident attribute values on lookup

[CharSyam]

I will check it soon. Thanks.

2026년 5월 26일 (화) 오전 9:09, Namjae Jeon <linkinjeon@kernel.org>님이 작성:
>
> On Tue, May 26, 2026 at 1:33 AM DaeMyung Kang <charsyam@gmail.com> wrote:
> >
> > ntfs_attr_find() and ntfs_external_attr_find() check that generic
> > resident attribute values fit in their attribute records and that
> > fixed-size resident values are large enough. For variable-length resident
> > formats, however, the fixed part is not enough: embedded length fields
> > can still point callers past the resident value.
> >
> > A crafted image can set a small resident $FILE_NAME value_length while
> > leaving file_name_length large. Callers then trust file_name_length and
> > read past the resident value when converting or comparing the name. This
> > was reproduced with a crafted image under KASAN as a slab-out-of-bounds
> > read from the kmalloc-1k MFT record copy. The stack included
> > ntfs_lookup(), ntfs_iget(), ntfs_read_locked_inode(), ntfs_attr_name_get(),
> > ntfs_ucstonls(), and utf16s_to_utf8s().
> >
> > Add a shared resident attribute value validator and use it before a
> > lookup path can return an attribute, including the AT_UNUSED enumeration
> > case where callers inspect returned attributes directly. The helper
> > validates resident value bounds, minimum value sizes, and variable-length
> > $FILE_NAME and $INDEX_ROOT fields, while preserving the fixed-size
> > checks for attributes such as $VOLUME_INFORMATION and $EA_INFORMATION.
> > Log corruption at the rejection point with the attribute name where
> > known.
> >
> > Reject non-resident $FILE_NAME records too: the format requires
> > $FILE_NAME to be resident and callers treat returned records as
> > resident.
> >
> > Type-specific format validation for $VOLUME_NAME is intentionally not
> > added to this helper, even though $VOLUME_NAME is a variable-length
> > resident attribute. Existing callers such as ntfs_write_volume_label()
> > treat a failed AT_VOLUME_NAME lookup as an absent label and
> > unconditionally add a new record afterwards. If the helper rejected
> > odd-length labels at lookup time, the corrupt record would stay in
> > place and the caller would append a new record next to it, producing
> > two $VOLUME_NAME attributes on disk. Caller-side cleanup must precede
> > any lookup-time rejection of malformed $VOLUME_NAME payloads, so the
> > type-specific check is left for a follow-up patch. The generic resident
> > bounds checks (value_offset / value_length / attr_len) still apply to
> > $VOLUME_NAME, and its entry in the type-name table is kept so that
> > those failures log with the attribute name.
> >
> > Fixes: 6ceb4cc81ef3 ("ntfs: add bound checking to ntfs_attr_find")
> > Signed-off-by: DaeMyung Kang <charsyam@gmail.com>
> > ---
> > Patch 1/2 of the previous series ("ntfs: free link name from
> > ntfs_name_cache") has been applied to ntfs-next; this v2 contains only
> > the remaining patch, with the resident attribute validation reworked per
> > review feedback.
> >
> > Changes since v1:
> > - Refactor resident attribute validation into a shared helper
> >   ntfs_attr_value_is_valid() used by both ntfs_attr_find() and
> >   ntfs_external_attr_find(), and remove the duplicated inline check
> >   from ntfs_external_attr_find().
> > - Split out resident header/value bounds extraction into
> >   ntfs_resident_attr_value_get(), returning a small view struct so the
> >   top-level orchestration reads as: get view -> common min check ->
> >   per-type format check.
> > - Validate before the AT_UNUSED enumeration path in ntfs_attr_find()
> >   can return the attribute to callers that inspect it directly.
> > - Reject non-resident $FILE_NAME records: the format requires
> >   $FILE_NAME to be resident and callers treat returned records as
> >   resident.
> > - Extend type-specific validation beyond $FILE_NAME to $INDEX_ROOT
> >   (entries_offset / index_length / allocated_size bounds and 8-byte
> >   alignment); add $INDEX_ROOT to ntfs_resident_attr_min_value_length().
> > - Type-specific format validation for $VOLUME_NAME is intentionally
> >   not added; see the commit message for the caller-side reason.
> >   Generic resident bounds checks still apply, and the $VOLUME_NAME
> >   entry in the type-name table is kept so that those failures log
> >   with a readable attribute name.
> > - Log corruption at the rejection point with the attribute type name
> >   via a new ntfs_attr_type_name() helper, matching the existing
> >   attribute-name corruption message in ntfs_attr_find().
> Please check xfstests generic/001 test
> failure.(https://github.com/namjaejeon/linux-ntfs/actions/runs/26424312505/job/77785594232)
>
> FSTYP -- ntfs
> 570PLATFORM -- Linux/x86_64 runnervmg397c 6.17.0-1013-azure
> #13~24.04.1-Ubuntu SMP Wed Apr 15 16:52:17 UTC 2026
> 571MKFS_OPTIONS -- -q --partition-start=0 --sectors-per-track=0
> --heads=0 /dev/loop21
> 572MOUNT_OPTIONS -- /dev/loop21 /mnt/scratch
> 573
> 574generic/001 _check_generic_filesystem: filesystem on /dev/loop20 is
> inconsistent
> 575(see /home/runner/work/linux-ntfs/linux-ntfs/exfat-testsuites/xfstests-exfat/results//generic/001.full
> for details)
> 576Trying to repair broken TEST_DEV file system
> 577- output mismatch (see
> /home/runner/work/linux-ntfs/linux-ntfs/exfat-testsuites/xfstests-exfat/results//generic/001.out.bad)
> 578 --- tests/generic/001.out 2025-01-14 04:54:54.000000000 +0000
> 579 +++ /home/runner/work/linux-ntfs/linux-ntfs/exfat-testsuites/xfstests-exfat/results//generic/001.out.bad
> 2026-05-26 00:01:44.663029196 +0000
> 580 @@ -7,3 +7,4 @@
> 581 iter 4 chain ... check ....................................
> 582 iter 5 chain ... check ....................................
> 583 cleanup
> 584 +rm: cannot remove '/mnt/test/83726/sub': Input/output error
> 585 ...
> 586 (Run 'diff -u
> /home/runner/work/linux-ntfs/linux-ntfs/exfat-testsuites/xfstests-exfat/tests/generic/001.out
> /home/runner/work/linux-ntfs/linux-ntfs/exfat-testsuites/xfstests-exfat/results//generic/001.out.bad'
> to see the entire diff)
> 587Ran: generic/001
> 588Failures: generic/001
> 589Failed 1 of 1 tests

[PATCH v3] ntfs: validate resident attribute values on lookup

[DaeMyung Kang]

ntfs_attr_find() and ntfs_external_attr_find() check that generic
resident attribute values fit in their attribute records and that
fixed-size resident values are large enough. For variable-length resident
formats, however, the fixed part is not enough: embedded length fields
can still point callers past the resident value.

A crafted image can set a small resident $FILE_NAME value_length while
leaving file_name_length large. Callers then trust file_name_length and
read past the resident value when converting or comparing the name. This
was reproduced with a crafted image under KASAN as a slab-out-of-bounds
read from the kmalloc-1k MFT record copy. The stack included
ntfs_lookup(), ntfs_iget(), ntfs_read_locked_inode(), ntfs_attr_name_get(),
ntfs_ucstonls(), and utf16s_to_utf8s().

Add a shared resident attribute value validator and use it before a
lookup path can return an attribute, including the AT_UNUSED enumeration
case where callers inspect returned attributes directly. The helper
validates resident value bounds, minimum value sizes, and the
variable-length $FILE_NAME field, while preserving the fixed-size checks
for attributes such as $VOLUME_INFORMATION and $EA_INFORMATION. Log
corruption at the rejection point with the attribute name where known.

Reject non-resident $FILE_NAME records too: the format requires
$FILE_NAME to be resident and callers treat returned records as
resident.

Type-specific format validation for $VOLUME_NAME is intentionally not
added to this helper, even though $VOLUME_NAME is a variable-length
resident attribute. Existing callers such as ntfs_write_volume_label()
treat a failed AT_VOLUME_NAME lookup as an absent label and
unconditionally add a new record afterwards. If the helper rejected
odd-length labels at lookup time, the corrupt record would stay in
place and the caller would append a new record next to it, producing
two $VOLUME_NAME attributes on disk. Caller-side cleanup must precede
any lookup-time rejection of malformed $VOLUME_NAME payloads, so the
type-specific check is left for a follow-up patch. The generic resident
bounds checks (value_offset / value_length / attr_len) still apply to
$VOLUME_NAME, and its entry in the type-name table is kept so that
those failures log with the attribute name.

Do not add $INDEX_ROOT-specific value validation in this change. Testing
with stricter checks showed an existing ntfs_ir_truncate() shrink
ordering bug: the resident value_length is shrunk before the root index
allocated_size is updated, so a relookup can observe allocated_size
beyond the now-smaller resident value and fail. Fix that ordering before
adding the $INDEX_ROOT-specific minimum-size and structural checks.

Fixes: 6ceb4cc81ef3 ("ntfs: add bound checking to ntfs_attr_find")
Signed-off-by: DaeMyung Kang <charsyam@gmail.com>
---
Patch 1/2 of the previous series ("ntfs: free link name from
ntfs_name_cache") has been applied to ntfs-next; this v3 contains only
the remaining patch, with the resident attribute validation reworked per
review feedback.

Changes since v2:
- Drop $INDEX_ROOT-specific validation from this patch, including both
  the minimum-size table entry and the structural checks. QEMU testing
  with xfstests generic/001 showed that those checks expose an existing
  ntfs_ir_truncate() shrink ordering bug, where value_length is reduced
  before allocated_size is synchronized. The same test passes when
  allocated_size is lowered before shrinking value_length, with the
  $INDEX_ROOT-specific validation enabled. That ordering issue will be
  fixed in a separate patch, after which the $INDEX_ROOT-specific
  validation can be reintroduced in a follow-up.

Changes since v1:
- Refactor resident attribute validation into a shared helper
  ntfs_attr_value_is_valid() used by both ntfs_attr_find() and
  ntfs_external_attr_find(), and remove the duplicated inline check
  from ntfs_external_attr_find().
- Split out resident header/value bounds extraction into
  ntfs_resident_attr_value_get(), returning a small view struct so the
  top-level orchestration reads as: get view -> common min check ->
  per-type format check.
- Validate before the AT_UNUSED enumeration path in ntfs_attr_find()
  can return the attribute to callers that inspect it directly.
- Reject non-resident $FILE_NAME records: the format requires
  $FILE_NAME to be resident and callers treat returned records as
  resident.
- Type-specific format validation for $VOLUME_NAME is intentionally
  not added; see the commit message for the caller-side reason.
  Generic resident bounds checks still apply, and the $VOLUME_NAME
  entry in the type-name table is kept so that those failures log
  with a readable attribute name.
- Log corruption at the rejection point with the attribute type name
  via a new ntfs_attr_type_name() helper, matching the existing
  attribute-name corruption message in ntfs_attr_find().

 fs/ntfs/attrib.c | 148 +++++++++++++++++++++++++++++++++++++++++++------------
 1 file changed, 117 insertions(+), 31 deletions(-)

diff --git a/fs/ntfs/attrib.c b/fs/ntfs/attrib.c
index 421c6cdcbb53..382d1b6e877e 100644
--- a/fs/ntfs/attrib.c
+++ b/fs/ntfs/attrib.c
@@ -595,6 +595,112 @@ static u32 ntfs_resident_attr_min_value_length(const __le32 type)
 	}
 }
 
+static const char *ntfs_attr_type_name(const __le32 type)
+{
+	switch (type) {
+	case AT_STANDARD_INFORMATION:
+		return "$STANDARD_INFORMATION";
+	case AT_FILE_NAME:
+		return "$FILE_NAME";
+	case AT_VOLUME_NAME:
+		return "$VOLUME_NAME";
+	case AT_VOLUME_INFORMATION:
+		return "$VOLUME_INFORMATION";
+	case AT_INDEX_ROOT:
+		return "$INDEX_ROOT";
+	case AT_EA_INFORMATION:
+		return "$EA_INFORMATION";
+	default:
+		return NULL;
+	}
+}
+
+static bool ntfs_file_name_attr_value_is_valid(const u8 *value, const u32 value_length)
+{
+	const struct file_name_attr *fn;
+	u32 file_name_size;
+
+	if (value_length < ntfs_resident_attr_min_value_length(AT_FILE_NAME))
+		return false;
+
+	fn = (const struct file_name_attr *)value;
+	file_name_size = fn->file_name_length * sizeof(__le16);
+
+	return file_name_size <=
+			value_length - offsetof(struct file_name_attr, file_name);
+}
+
+struct ntfs_resident_attr_value {
+	const u8 *data;
+	u32 len;
+};
+
+static bool ntfs_resident_attr_value_get(const struct attr_record *a,
+					 struct ntfs_resident_attr_value *value)
+{
+	u32 attr_len;
+	u16 value_offset;
+
+	attr_len = le32_to_cpu(a->length);
+	if (attr_len < offsetof(struct attr_record, data.resident.reserved) +
+			sizeof(a->data.resident.reserved))
+		return false;
+
+	value->len = le32_to_cpu(a->data.resident.value_length);
+	value_offset = le16_to_cpu(a->data.resident.value_offset);
+
+	if (value->len > attr_len || value_offset > attr_len - value->len)
+		return false;
+
+	value->data = (const u8 *)a + value_offset;
+	return true;
+}
+
+static bool ntfs_attr_value_is_valid(struct ntfs_volume *vol,
+				     const struct attr_record *a,
+				     const u64 mft_no)
+{
+	struct ntfs_resident_attr_value value;
+	const char *type_name;
+	u32 min_len;
+
+	if (a->non_resident) {
+		if (a->type != AT_FILE_NAME)
+			return true;
+		ntfs_error(vol->sb,
+			   "Corrupt non-resident $FILE_NAME attribute in MFT record %llu\n",
+			   mft_no);
+		return false;
+	}
+
+	if (!ntfs_resident_attr_value_get(a, &value))
+		goto corrupt;
+
+	min_len = ntfs_resident_attr_min_value_length(a->type);
+	if (min_len && value.len < min_len)
+		goto corrupt;
+
+	switch (a->type) {
+	case AT_FILE_NAME:
+		if (!ntfs_file_name_attr_value_is_valid(value.data, value.len))
+			goto corrupt;
+		break;
+	}
+	return true;
+
+corrupt:
+	type_name = ntfs_attr_type_name(a->type);
+	if (type_name)
+		ntfs_error(vol->sb,
+			   "Corrupt resident %s attribute in MFT record %llu\n",
+			   type_name, mft_no);
+	else
+		ntfs_error(vol->sb,
+			   "Corrupt resident %#x attribute in MFT record %llu\n",
+			   le32_to_cpu(a->type), mft_no);
+	return false;
+}
+
 /*
  * ntfs_attr_find - find (next) attribute in mft record
  * @type:	attribute type to find
@@ -705,8 +811,11 @@ static int ntfs_attr_find(const __le32 type, const __le16 *name,
 			}
 		}
 
-		if (type == AT_UNUSED)
+		if (type == AT_UNUSED) {
+			if (!ntfs_attr_value_is_valid(vol, a, ctx->ntfs_ino->mft_no))
+				break;
 			return 0;
+		}
 		if (a->type != type)
 			continue;
 		/*
@@ -747,24 +856,11 @@ static int ntfs_attr_find(const __le32 type, const __le16 *name,
 			}
 		}
 
-		 /* Validate attribute's value offset/length */
-		if (!a->non_resident) {
-			u32 min_len;
-			u32 value_length = le32_to_cpu(a->data.resident.value_length);
-			u16 value_offset = le16_to_cpu(a->data.resident.value_offset);
-
-			if (value_length > le32_to_cpu(a->length) ||
-			    value_offset > le32_to_cpu(a->length) - value_length)
-				break;
+		if (!ntfs_attr_value_is_valid(vol, a, ctx->ntfs_ino->mft_no))
+			break;
 
-			min_len = ntfs_resident_attr_min_value_length(a->type);
-			if (min_len && value_length < min_len) {
-				ntfs_error(vol->sb,
-					   "Too small %#x resident attribute value in MFT record %lld\n",
-					   le32_to_cpu(a->type), (long long)ctx->ntfs_ino->mft_no);
-				break;
-			}
-		} else {
+		/* Validate non-resident mapping-pairs fields. */
+		if (a->non_resident) {
 			u32 min_len;
 			u16 mp_offset;
 
@@ -1252,6 +1348,9 @@ static int ntfs_external_attr_find(const __le32 type,
 
 		ctx->attr = a;
 
+		if (!ntfs_attr_value_is_valid(vol, a, ctx->ntfs_ino->mft_no))
+			break;
+
 		if (a->non_resident) {
 			u32 min_len;
 			u16 mp_offset;
@@ -1279,19 +1378,6 @@ static int ntfs_external_attr_find(const __le32 type,
 			u32 value_length = le32_to_cpu(a->data.resident.value_length);
 			u16 value_offset = le16_to_cpu(a->data.resident.value_offset);
 
-			if (attr_len < offsetof(struct attr_record, data.resident.reserved) +
-					sizeof(a->data.resident.reserved))
-				break;
-			if (value_length > attr_len || value_offset > attr_len - value_length)
-				break;
-
-			value_length = ntfs_resident_attr_min_value_length(a->type);
-			if (value_length && le32_to_cpu(a->data.resident.value_length) <
-			    value_length) {
-				pr_err("Too small resident attribute value in MFT record %lld, type %#x\n",
-				       (long long)ctx->ntfs_ino->mft_no, a->type);
-				break;
-			}
 			if (value_length == val_len &&
 			    !memcmp((u8 *)a + value_offset, val, val_len)) {
 attr_found:
-- 
2.43.0

Re: [PATCH v3] ntfs: validate resident attribute values on lookup

[Namjae Jeon]

> Type-specific format validation for $VOLUME_NAME is intentionally not
> added to this helper, even though $VOLUME_NAME is a variable-length
> resident attribute. Existing callers such as ntfs_write_volume_label()
> treat a failed AT_VOLUME_NAME lookup as an absent label and
> unconditionally add a new record afterwards. If the helper rejected
> odd-length labels at lookup time, the corrupt record would stay in
> place and the caller would append a new record next to it, producing
> two $VOLUME_NAME attributes on disk. Caller-side cleanup must precede
> any lookup-time rejection of malformed $VOLUME_NAME payloads, so the
> type-specific check is left for a follow-up patch. The generic resident
> bounds checks (value_offset / value_length / attr_len) still apply to
> $VOLUME_NAME, and its entry in the type-name table is kept so that
> those failures log with the attribute name.
We can make ntfs_attr_find() return -EIO instead of -ENOENT when it
detects a corrupted resident attribute value. And callers should check
errno not to add duplicated $VOLUME_NAME.

>
> Do not add $INDEX_ROOT-specific value validation in this change. Testing
> with stricter checks showed an existing ntfs_ir_truncate() shrink
> ordering bug: the resident value_length is shrunk before the root index
> allocated_size is updated, so a relookup can observe allocated_size
> beyond the now-smaller resident value and fail. Fix that ordering before
> adding the $INDEX_ROOT-specific minimum-size and structural checks.
please provide patch series including ntfs_ir_truncate bug, index root
validation, volume name, and this patch.

> +static const char *ntfs_attr_type_name(const __le32 type)
> +{
> +       switch (type) {
> +       case AT_STANDARD_INFORMATION:
> +               return "$STANDARD_INFORMATION";
> +       case AT_FILE_NAME:
> +               return "$FILE_NAME";
> +       case AT_VOLUME_NAME:
> +               return "$VOLUME_NAME";
> +       case AT_VOLUME_INFORMATION:
> +               return "$VOLUME_INFORMATION";
> +       case AT_INDEX_ROOT:
> +               return "$INDEX_ROOT";
> +       case AT_EA_INFORMATION:
> +               return "$EA_INFORMATION";
> +       default:
> +               return NULL;
> +       }
> +}
We can identify the attribute just by printing its type, without
needing to print the attribute name.
Thanks!

Re: [PATCH v3] ntfs: validate resident attribute values on lookup

[Hyunchul Lee]

Hi DaeMyung,

2026년 5월 26일 (화) 오후 10:31, DaeMyung Kang <charsyam@gmail.com>님이 작성:
>
> ntfs_attr_find() and ntfs_external_attr_find() check that generic
> resident attribute values fit in their attribute records and that
> fixed-size resident values are large enough. For variable-length resident
> formats, however, the fixed part is not enough: embedded length fields
> can still point callers past the resident value.
>
> A crafted image can set a small resident $FILE_NAME value_length while
> leaving file_name_length large. Callers then trust file_name_length and
> read past the resident value when converting or comparing the name. This
> was reproduced with a crafted image under KASAN as a slab-out-of-bounds
> read from the kmalloc-1k MFT record copy. The stack included
> ntfs_lookup(), ntfs_iget(), ntfs_read_locked_inode(), ntfs_attr_name_get(),
> ntfs_ucstonls(), and utf16s_to_utf8s().
>
> Add a shared resident attribute value validator and use it before a
> lookup path can return an attribute, including the AT_UNUSED enumeration
> case where callers inspect returned attributes directly. The helper
> validates resident value bounds, minimum value sizes, and the
> variable-length $FILE_NAME field, while preserving the fixed-size checks
> for attributes such as $VOLUME_INFORMATION and $EA_INFORMATION. Log
> corruption at the rejection point with the attribute name where known.
>
> Reject non-resident $FILE_NAME records too: the format requires
> $FILE_NAME to be resident and callers treat returned records as
> resident.
>
> Type-specific format validation for $VOLUME_NAME is intentionally not
> added to this helper, even though $VOLUME_NAME is a variable-length
> resident attribute. Existing callers such as ntfs_write_volume_label()
> treat a failed AT_VOLUME_NAME lookup as an absent label and
> unconditionally add a new record afterwards. If the helper rejected
> odd-length labels at lookup time, the corrupt record would stay in
> place and the caller would append a new record next to it, producing
> two $VOLUME_NAME attributes on disk. Caller-side cleanup must precede
> any lookup-time rejection of malformed $VOLUME_NAME payloads, so the
> type-specific check is left for a follow-up patch. The generic resident
> bounds checks (value_offset / value_length / attr_len) still apply to
> $VOLUME_NAME, and its entry in the type-name table is kept so that
> those failures log with the attribute name.
>
> Do not add $INDEX_ROOT-specific value validation in this change. Testing
> with stricter checks showed an existing ntfs_ir_truncate() shrink
> ordering bug: the resident value_length is shrunk before the root index
> allocated_size is updated, so a relookup can observe allocated_size
> beyond the now-smaller resident value and fail. Fix that ordering before
> adding the $INDEX_ROOT-specific minimum-size and structural checks.
>
> Fixes: 6ceb4cc81ef3 ("ntfs: add bound checking to ntfs_attr_find")
> Signed-off-by: DaeMyung Kang <charsyam@gmail.com>
> ---
> Patch 1/2 of the previous series ("ntfs: free link name from
> ntfs_name_cache") has been applied to ntfs-next; this v3 contains only
> the remaining patch, with the resident attribute validation reworked per
> review feedback.
>
> Changes since v2:
> - Drop $INDEX_ROOT-specific validation from this patch, including both
>   the minimum-size table entry and the structural checks. QEMU testing
>   with xfstests generic/001 showed that those checks expose an existing
>   ntfs_ir_truncate() shrink ordering bug, where value_length is reduced
>   before allocated_size is synchronized. The same test passes when
>   allocated_size is lowered before shrinking value_length, with the
>   $INDEX_ROOT-specific validation enabled. That ordering issue will be
>   fixed in a separate patch, after which the $INDEX_ROOT-specific
>   validation can be reintroduced in a follow-up.
>
> Changes since v1:
> - Refactor resident attribute validation into a shared helper
>   ntfs_attr_value_is_valid() used by both ntfs_attr_find() and
>   ntfs_external_attr_find(), and remove the duplicated inline check
>   from ntfs_external_attr_find().
> - Split out resident header/value bounds extraction into
>   ntfs_resident_attr_value_get(), returning a small view struct so the
>   top-level orchestration reads as: get view -> common min check ->
>   per-type format check.
> - Validate before the AT_UNUSED enumeration path in ntfs_attr_find()
>   can return the attribute to callers that inspect it directly.
> - Reject non-resident $FILE_NAME records: the format requires
>   $FILE_NAME to be resident and callers treat returned records as
>   resident.
> - Type-specific format validation for $VOLUME_NAME is intentionally
>   not added; see the commit message for the caller-side reason.
>   Generic resident bounds checks still apply, and the $VOLUME_NAME
>   entry in the type-name table is kept so that those failures log
>   with a readable attribute name.
> - Log corruption at the rejection point with the attribute type name
>   via a new ntfs_attr_type_name() helper, matching the existing
>   attribute-name corruption message in ntfs_attr_find().
>
>  fs/ntfs/attrib.c | 148 +++++++++++++++++++++++++++++++++++++++++++------------
>  1 file changed, 117 insertions(+), 31 deletions(-)
>
> diff --git a/fs/ntfs/attrib.c b/fs/ntfs/attrib.c
> index 421c6cdcbb53..382d1b6e877e 100644
> --- a/fs/ntfs/attrib.c
> +++ b/fs/ntfs/attrib.c
> @@ -595,6 +595,112 @@ static u32 ntfs_resident_attr_min_value_length(const __le32 type)
>         }
>  }
>
> +static const char *ntfs_attr_type_name(const __le32 type)
> +{
> +       switch (type) {
> +       case AT_STANDARD_INFORMATION:
> +               return "$STANDARD_INFORMATION";
> +       case AT_FILE_NAME:
> +               return "$FILE_NAME";
> +       case AT_VOLUME_NAME:
> +               return "$VOLUME_NAME";
> +       case AT_VOLUME_INFORMATION:
> +               return "$VOLUME_INFORMATION";
> +       case AT_INDEX_ROOT:
> +               return "$INDEX_ROOT";
> +       case AT_EA_INFORMATION:
> +               return "$EA_INFORMATION";
> +       default:
> +               return NULL;
> +       }
> +}
> +
> +static bool ntfs_file_name_attr_value_is_valid(const u8 *value, const u32 value_length)
> +{
> +       const struct file_name_attr *fn;
> +       u32 file_name_size;
> +
> +       if (value_length < ntfs_resident_attr_min_value_length(AT_FILE_NAME))
> +               return false;
> +
> +       fn = (const struct file_name_attr *)value;
> +       file_name_size = fn->file_name_length * sizeof(__le16);
> +
> +       return file_name_size <=
> +                       value_length - offsetof(struct file_name_attr, file_name);
> +}
> +
> +struct ntfs_resident_attr_value {
> +       const u8 *data;
> +       u32 len;
> +};
> +
> +static bool ntfs_resident_attr_value_get(const struct attr_record *a,
> +                                        struct ntfs_resident_attr_value *value)
> +{
> +       u32 attr_len;
> +       u16 value_offset;
> +
> +       attr_len = le32_to_cpu(a->length);
> +       if (attr_len < offsetof(struct attr_record, data.resident.reserved) +
> +                       sizeof(a->data.resident.reserved))
> +               return false;
> +
> +       value->len = le32_to_cpu(a->data.resident.value_length);
> +       value_offset = le16_to_cpu(a->data.resident.value_offset);
> +
> +       if (value->len > attr_len || value_offset > attr_len - value->len)
> +               return false;
> +
> +       value->data = (const u8 *)a + value_offset;
> +       return true;
> +}
> +
> +static bool ntfs_attr_value_is_valid(struct ntfs_volume *vol,
> +                                    const struct attr_record *a,
> +                                    const u64 mft_no)
> +{
> +       struct ntfs_resident_attr_value value;
> +       const char *type_name;
> +       u32 min_len;
> +
> +       if (a->non_resident) {
> +               if (a->type != AT_FILE_NAME)
> +                       return true;
> +               ntfs_error(vol->sb,
> +                          "Corrupt non-resident $FILE_NAME attribute in MFT record %llu\n",
> +                          mft_no);
> +               return false;
> +       }
> +
> +       if (!ntfs_resident_attr_value_get(a, &value))
> +               goto corrupt;
> +
> +       min_len = ntfs_resident_attr_min_value_length(a->type);
> +       if (min_len && value.len < min_len)
> +               goto corrupt;
> +
> +       switch (a->type) {
> +       case AT_FILE_NAME:
> +               if (!ntfs_file_name_attr_value_is_valid(value.data, value.len))
> +                       goto corrupt;
> +               break;
> +       }
> +       return true;
> +
> +corrupt:
> +       type_name = ntfs_attr_type_name(a->type);
> +       if (type_name)
> +               ntfs_error(vol->sb,
> +                          "Corrupt resident %s attribute in MFT record %llu\n",
> +                          type_name, mft_no);
> +       else
> +               ntfs_error(vol->sb,
> +                          "Corrupt resident %#x attribute in MFT record %llu\n",
> +                          le32_to_cpu(a->type), mft_no);
> +       return false;
> +}
> +
>  /*
>   * ntfs_attr_find - find (next) attribute in mft record
>   * @type:      attribute type to find
> @@ -705,8 +811,11 @@ static int ntfs_attr_find(const __le32 type, const __le16 *name,
>                         }
>                 }
>
> -               if (type == AT_UNUSED)
> +               if (type == AT_UNUSED) {
> +                       if (!ntfs_attr_value_is_valid(vol, a, ctx->ntfs_ino->mft_no))
> +                               break;
>                         return 0;
> +               }
>                 if (a->type != type)
>                         continue;
>                 /*
> @@ -747,24 +856,11 @@ static int ntfs_attr_find(const __le32 type, const __le16 *name,
>                         }
>                 }
>
> -                /* Validate attribute's value offset/length */
> -               if (!a->non_resident) {
> -                       u32 min_len;
> -                       u32 value_length = le32_to_cpu(a->data.resident.value_length);
> -                       u16 value_offset = le16_to_cpu(a->data.resident.value_offset);
> -
> -                       if (value_length > le32_to_cpu(a->length) ||
> -                           value_offset > le32_to_cpu(a->length) - value_length)
> -                               break;
> +               if (!ntfs_attr_value_is_valid(vol, a, ctx->ntfs_ino->mft_no))
> +                       break;
>
> -                       min_len = ntfs_resident_attr_min_value_length(a->type);
> -                       if (min_len && value_length < min_len) {
> -                               ntfs_error(vol->sb,
> -                                          "Too small %#x resident attribute value in MFT record %lld\n",
> -                                          le32_to_cpu(a->type), (long long)ctx->ntfs_ino->mft_no);
> -                               break;
> -                       }
> -               } else {
> +               /* Validate non-resident mapping-pairs fields. */
> +               if (a->non_resident) {

How about moving this validation into ntfs_attr_value_is_valid() as
well? The function name
suggests that it validates both resident and non-resident attributes,
and since the same
validation logic also exists in ntfs_attr_external_find(), moving it
would help prevent
duplication.


>                         u32 min_len;
>                         u16 mp_offset;
>
> @@ -1252,6 +1348,9 @@ static int ntfs_external_attr_find(const __le32 type,
>
>                 ctx->attr = a;
>
> +               if (!ntfs_attr_value_is_valid(vol, a, ctx->ntfs_ino->mft_no))
> +                       break;
> +
>                 if (a->non_resident) {
>                         u32 min_len;
>                         u16 mp_offset;
> @@ -1279,19 +1378,6 @@ static int ntfs_external_attr_find(const __le32 type,
>                         u32 value_length = le32_to_cpu(a->data.resident.value_length);
>                         u16 value_offset = le16_to_cpu(a->data.resident.value_offset);
>
> -                       if (attr_len < offsetof(struct attr_record, data.resident.reserved) +
> -                                       sizeof(a->data.resident.reserved))
> -                               break;
> -                       if (value_length > attr_len || value_offset > attr_len - value_length)
> -                               break;
> -
> -                       value_length = ntfs_resident_attr_min_value_length(a->type);
> -                       if (value_length && le32_to_cpu(a->data.resident.value_length) <
> -                           value_length) {
> -                               pr_err("Too small resident attribute value in MFT record %lld, type %#x\n",
> -                                      (long long)ctx->ntfs_ino->mft_no, a->type);
> -                               break;
> -                       }
>                         if (value_length == val_len &&
>                             !memcmp((u8 *)a + value_offset, val, val_len)) {
>  attr_found:
> --
> 2.43.0



-- 
Thanks,
Hyunchul