[PATCH v4 0/6] ntfs: validate attribute values on lookup

[PATCH v4 0/6] ntfs: validate attribute values on lookup

[DaeMyung Kang]

This v4 supersedes the previous single-patch v3. Per review, it expands
the lookup-time attribute value validation into a series so $VOLUME_NAME
and $INDEX_ROOT can be validated safely.

The first patch keeps the original $FILE_NAME corruption fix and moves
the duplicated non-resident mapping-pairs metadata checks into the common
attribute value validator. It also keeps resident value matching in the
external attribute lookup path on the actual value length and fixes the
matching attrlist duplicate check for resident attributes. The next
patches prepare the volume label write and mount read paths to cope with
a rejected $VOLUME_NAME, add $VOLUME_NAME validation, fix the
ntfs_ir_truncate() shrink ordering issue, and then enable $INDEX_ROOT
validation.

Changes since v3:
- Expand the single patch into a 6-patch series as requested.
- Keep corruption messages using numeric attribute types and drop the
  attribute type name helper.
- Move non-resident mapping-pairs metadata validation into the shared
  ntfs_attr_value_is_valid() helper.
- Preserve resident @val matching in ntfs_external_attr_find() using the
  actual resident value length, and avoid reading the non-resident
  lowest_vcn union member when checking resident attrlist duplicates.
- Make ntfs_write_volume_label() add a replacement only after successful
  removal or after -ENOENT, and propagate other lookup/removal errors.
- Reinitialize the mount-time search context before the
  $VOLUME_INFORMATION lookup, since a rejected $VOLUME_NAME leaves the
  context in an undefined state.
- Add $VOLUME_NAME-specific validation, and reject non-resident
  $VOLUME_NAME records like non-resident $FILE_NAME.
- Include the ntfs_ir_truncate() shrink ordering fix.
- Add $INDEX_ROOT-specific validation on top of the shrink fix.

DaeMyung Kang (6):
  ntfs: validate attribute values on lookup
  ntfs: do not replace volume name after lookup errors
  ntfs: reinit search context before volume information lookup
  ntfs: validate resident volume name values on lookup
  ntfs: update index root allocated size before shrink
  ntfs: validate resident index root values on lookup

 fs/ntfs/attrib.c   | 203 +++++++++++++++++++++++++++++++++++++----------------
 fs/ntfs/attrlist.c |  11 ++-
 fs/ntfs/index.c    |  18 ++++-
 fs/ntfs/super.c    |  12 +++-
 4 files changed, 174 insertions(+), 70 deletions(-)


base-commit: 8553f258a57607dbde0b8baab47459aa28eb92f7
-- 
2.43.0

[PATCH v4 1/6] ntfs: validate 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 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 resident value sizes, variable-length
$FILE_NAME fields, and non-resident mapping-pairs metadata that was
previously checked separately in both lookup paths.

This also preserves the intended resident @val matching semantics in the
external attribute lookup path. The old duplicated validation block
overwrote the actual resident value length with the type-specific minimum
length before comparing @val, so variable-length resident values could
fail to match even when the bytes were identical. Keep the comparison on
the actual value length, and make ntfs_attrlist_entry_add() compare
resident attributes with lowest_vcn zero instead of reading the
non-resident union member after a successful resident match.

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   | 160 +++++++++++++++++++++++++++++++++--------------------
 fs/ntfs/attrlist.c |  11 +++-
 2 files changed, 107 insertions(+), 64 deletions(-)

diff --git a/fs/ntfs/attrlist.c b/fs/ntfs/attrlist.c
index c2594d4c83b0..afb13038ba42 100644
--- a/fs/ntfs/attrlist.c
+++ b/fs/ntfs/attrlist.c
@@ -118,6 +118,7 @@ int ntfs_attrlist_entry_add(struct ntfs_inode *ni, struct attr_record *attr)
 	int entry_len, entry_offset, err;
 	struct mft_record *ni_mrec;
 	u8 *old_al;
+	__le64 lowest_vcn;
 
 	if (!ni || !attr) {
 		ntfs_debug("Invalid arguments.\n");
@@ -158,17 +159,21 @@ int ntfs_attrlist_entry_add(struct ntfs_inode *ni, struct attr_record *attr)
 		ntfs_error(ni->vol->sb, "Failed to get search context");
 		goto err_out;
 	}
+	if (attr->non_resident)
+		lowest_vcn = attr->data.non_resident.lowest_vcn;
+	else
+		lowest_vcn = 0;
 
 	err = ntfs_attr_lookup(attr->type, (attr->name_length) ? (__le16 *)
 			((u8 *)attr + le16_to_cpu(attr->name_offset)) :
 			AT_UNNAMED, attr->name_length, CASE_SENSITIVE,
-			(attr->non_resident) ? le64_to_cpu(attr->data.non_resident.lowest_vcn) :
-			0, (attr->non_resident) ? NULL : ((u8 *)attr +
+			le64_to_cpu(lowest_vcn),
+			(attr->non_resident) ? NULL : ((u8 *)attr +
 			le16_to_cpu(attr->data.resident.value_offset)), (attr->non_resident) ?
 			0 : le32_to_cpu(attr->data.resident.value_length), ctx);
 	if (!err) {
 		/* Found some extent, check it to be before new extent. */
-		if (ctx->al_entry->lowest_vcn == attr->data.non_resident.lowest_vcn) {
+		if (ctx->al_entry->lowest_vcn == lowest_vcn) {
 			err = -EEXIST;
 			ntfs_debug("Such attribute already present in the attribute list.\n");
 			ntfs_attr_put_search_ctx(ctx);
diff --git a/fs/ntfs/attrib.c b/fs/ntfs/attrib.c
index 421c6cdcbb53..98e0b8ea2edd 100644
--- a/fs/ntfs/attrib.c
+++ b/fs/ntfs/attrib.c
@@ -595,6 +595,97 @@ static u32 ntfs_resident_attr_min_value_length(const __le32 type)
 	}
 }
 
+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;
+
+	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_non_resident_attr_value_is_valid(const struct attr_record *a)
+{
+	u32 attr_len;
+	u32 min_len;
+	u16 mp_offset;
+
+	attr_len = le32_to_cpu(a->length);
+	min_len = offsetof(struct attr_record, data.non_resident.initialized_size) +
+		  sizeof(a->data.non_resident.initialized_size);
+	if (attr_len < min_len)
+		return false;
+
+	mp_offset = le16_to_cpu(a->data.non_resident.mapping_pairs_offset);
+	return mp_offset >= min_len && mp_offset <= attr_len;
+}
+
+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;
+	u32 min_len;
+
+	if (a->non_resident) {
+		if (a->type == AT_FILE_NAME)
+			goto corrupt;
+		if (!ntfs_non_resident_attr_value_is_valid(a))
+			goto corrupt;
+		return true;
+	}
+
+	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:
+	ntfs_error(vol->sb,
+		   "Corrupt %#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 +796,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,37 +841,8 @@ 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;
-
-			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 {
-			u32 min_len;
-			u16 mp_offset;
-
-			min_len = offsetof(struct attr_record, data.non_resident.initialized_size) +
-				  sizeof(a->data.non_resident.initialized_size);
-			if (le32_to_cpu(a->length) < min_len)
-				break;
-
-			mp_offset = le16_to_cpu(a->data.non_resident.mapping_pairs_offset);
-			if (mp_offset < min_len ||
-			    mp_offset > le32_to_cpu(a->length))
-				break;
-		}
+		if (!ntfs_attr_value_is_valid(vol, a, ctx->ntfs_ino->mft_no))
+			break;
 
 		/*
 		 * The names match or @name not present and attribute is
@@ -1252,22 +1317,8 @@ static int ntfs_external_attr_find(const __le32 type,
 
 		ctx->attr = a;
 
-		if (a->non_resident) {
-			u32 min_len;
-			u16 mp_offset;
-
-			min_len = offsetof(struct attr_record,
-					   data.non_resident.initialized_size) +
-				  sizeof(a->data.non_resident.initialized_size);
-
-			if (le32_to_cpu(a->length) < min_len)
-				break;
-
-			mp_offset =
-				le16_to_cpu(a->data.non_resident.mapping_pairs_offset);
-			if (mp_offset < min_len || mp_offset > attr_len)
-				break;
-		}
+		if (!ntfs_attr_value_is_valid(vol, a, ctx->ntfs_ino->mft_no))
+			break;
 
 		/*
 		 * If no @val specified or @val specified and it matches, we
@@ -1279,19 +1330,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

[PATCH v4 2/6] ntfs: do not replace volume name after lookup errors

[DaeMyung Kang]

ntfs_write_volume_label() removes an existing $VOLUME_NAME attribute and
then adds the replacement. The old code only distinguished lookup success
from all other results, so any lookup error was treated like an absent
label and the add path still ran.

That is unsafe once lookup-time validation rejects corrupt $VOLUME_NAME
records with -EIO: the corrupt record would remain in place and a second
$VOLUME_NAME record could be appended next to it.

Only add the replacement after the old label was removed successfully or
after lookup returned -ENOENT. Propagate all other lookup errors, and
also stop if removing the old attribute fails.

Signed-off-by: DaeMyung Kang <charsyam@gmail.com>
---
 fs/ntfs/super.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/fs/ntfs/super.c b/fs/ntfs/super.c
index a2648d1c8fd0..140b8c8a811e 100644
--- a/fs/ntfs/super.c
+++ b/fs/ntfs/super.c
@@ -452,10 +452,15 @@ int ntfs_write_volume_label(struct ntfs_volume *vol, char *label)
 		goto out;
 	}
 
-	if (!ntfs_attr_lookup(AT_VOLUME_NAME, NULL, 0, 0, 0, NULL, 0,
-			     ctx))
-		ntfs_attr_record_rm(ctx);
+	ret = ntfs_attr_lookup(AT_VOLUME_NAME, NULL, 0, 0, 0, NULL, 0,
+			       ctx);
+	if (!ret)
+		ret = ntfs_attr_record_rm(ctx);
+	else if (ret == -ENOENT)
+		ret = 0;
 	ntfs_attr_put_search_ctx(ctx);
+	if (ret)
+		goto out;
 
 	ret = ntfs_resident_attr_record_add(vol_ni, AT_VOLUME_NAME, AT_UNNAMED, 0,
 					    (u8 *)uname, uname_len * sizeof(__le16), 0);
-- 
2.43.0

[PATCH v4 3/6] ntfs: reinit search context before volume information lookup

[DaeMyung Kang]

On mount the volume inode is searched for $VOLUME_NAME and then, reusing
the same search context, for $VOLUME_INFORMATION. The $VOLUME_NAME lookup
is optional and its result is otherwise ignored.

Once lookup-time validation can reject a corrupt $VOLUME_NAME with -EIO,
the search context is left in an undefined state: ntfs_attr_find()
documents that on an actual error @ctx->attr is undefined. Continuing the
$VOLUME_INFORMATION search from that context is not contractually valid.

Reinitialize the search context before the $VOLUME_INFORMATION lookup so
it always starts from a well-defined state regardless of the
$VOLUME_NAME lookup outcome.

Signed-off-by: DaeMyung Kang <charsyam@gmail.com>
---
 fs/ntfs/super.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/ntfs/super.c b/fs/ntfs/super.c
index 140b8c8a811e..79b530280d87 100644
--- a/fs/ntfs/super.c
+++ b/fs/ntfs/super.c
@@ -1537,6 +1537,7 @@ static bool load_system_files(struct ntfs_volume *vol)
 			vol->volume_label = NULL;
 	}
 
+	ntfs_attr_reinit_search_ctx(ctx);
 	if (ntfs_attr_lookup(AT_VOLUME_INFORMATION, NULL, 0, 0, 0, NULL, 0,
 			ctx) || ctx->attr->non_resident || ctx->attr->flags) {
 		ntfs_attr_put_search_ctx(ctx);
-- 
2.43.0

[PATCH v4 4/6] ntfs: validate resident volume name values on lookup

[DaeMyung Kang]

The shared lookup-time attribute validator now has a safe caller path for
$VOLUME_NAME corruption: ntfs_write_volume_label() no longer treats
lookup errors as an absent label, and the mount path reinitializes its
search context before continuing to $VOLUME_INFORMATION.

Add $VOLUME_NAME-specific resident value validation. A volume name is
stored as a UTF-16LE string, so reject odd byte lengths, and reject
values longer than the NTFS volume label limit. Empty labels remain
valid.

Also reject non-resident $VOLUME_NAME records. $VOLUME_NAME is required
to be resident, like $FILE_NAME; a crafted non-resident record would
otherwise pass lookup and ntfs_write_volume_label() would remove it as if
it were a normal resident attribute.

Signed-off-by: DaeMyung Kang <charsyam@gmail.com>
---
 fs/ntfs/attrib.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/fs/ntfs/attrib.c b/fs/ntfs/attrib.c
index 98e0b8ea2edd..7e293b85ad19 100644
--- a/fs/ntfs/attrib.c
+++ b/fs/ntfs/attrib.c
@@ -607,6 +607,14 @@ static bool ntfs_file_name_attr_value_is_valid(const u8 *value, const u32 value_
 			value_length - offsetof(struct file_name_attr, file_name);
 }
 
+static bool ntfs_volume_name_attr_value_is_valid(const u32 value_length)
+{
+	if (value_length & 1)
+		return false;
+
+	return value_length <= NTFS_MAX_LABEL_LEN * sizeof(__le16);
+}
+
 struct ntfs_resident_attr_value {
 	const u8 *data;
 	u32 len;
@@ -657,7 +665,7 @@ static bool ntfs_attr_value_is_valid(struct ntfs_volume *vol,
 	u32 min_len;
 
 	if (a->non_resident) {
-		if (a->type == AT_FILE_NAME)
+		if (a->type == AT_FILE_NAME || a->type == AT_VOLUME_NAME)
 			goto corrupt;
 		if (!ntfs_non_resident_attr_value_is_valid(a))
 			goto corrupt;
@@ -676,6 +684,10 @@ static bool ntfs_attr_value_is_valid(struct ntfs_volume *vol,
 		if (!ntfs_file_name_attr_value_is_valid(value.data, value.len))
 			goto corrupt;
 		break;
+	case AT_VOLUME_NAME:
+		if (!ntfs_volume_name_attr_value_is_valid(value.len))
+			goto corrupt;
+		break;
 	}
 	return true;
 
-- 
2.43.0

[PATCH v4 5/6] ntfs: update index root allocated size before shrink

[DaeMyung Kang]

ntfs_ir_truncate() currently shrinks the resident $INDEX_ROOT value first
and only updates index.allocated_size after re-looking up the attribute.
During that relookup, the resident value_length can already be smaller
while index.allocated_size still contains the old larger size.

That leaves a transiently inconsistent $INDEX_ROOT layout and prevents
lookup-time $INDEX_ROOT validation from being enabled: validation can
correctly reject allocated_size extending past the newly shrunk resident
value.

When shrinking, lower index.allocated_size before shrinking value_length.
If the truncate fails, restore the old allocated_size. Keep the existing
grow ordering because the old allocated_size remains within the enlarged
resident value until it is updated after the relookup. The shrink path is
safe because the new value_length still covers struct index_root, so the
index.allocated_size field remains present while it is updated first.

Signed-off-by: DaeMyung Kang <charsyam@gmail.com>
---
 fs/ntfs/index.c | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/fs/ntfs/index.c b/fs/ntfs/index.c
index 146e011c1a41..068c68a0f9cf 100644
--- a/fs/ntfs/index.c
+++ b/fs/ntfs/index.c
@@ -1280,9 +1280,16 @@ static int ntfs_ir_reparent(struct ntfs_index_context *icx)
 static int ntfs_ir_truncate(struct ntfs_index_context *icx, int data_size)
 {
 	int ret;
+	u32 old_allocated_size;
+	bool shrink;
 
 	ntfs_debug("Entering\n");
 
+	old_allocated_size = le32_to_cpu(icx->ir->index.allocated_size);
+	shrink = data_size < old_allocated_size;
+	if (shrink)
+		icx->ir->index.allocated_size = cpu_to_le32(data_size);
+
 	/*
 	 *  INDEX_ROOT must be resident and its entries can be moved to
 	 *  struct index_block, so ENOSPC isn't a real error.
@@ -1294,9 +1301,14 @@ static int ntfs_ir_truncate(struct ntfs_index_context *icx, int data_size)
 		if (!icx->ir)
 			return -ENOENT;
 
-		icx->ir->index.allocated_size = cpu_to_le32(data_size);
-	} else if (ret != -ENOSPC)
-		ntfs_error(icx->idx_ni->vol->sb, "Failed to truncate INDEX_ROOT");
+		if (!shrink)
+			icx->ir->index.allocated_size = cpu_to_le32(data_size);
+	} else {
+		if (shrink)
+			icx->ir->index.allocated_size = cpu_to_le32(old_allocated_size);
+		if (ret != -ENOSPC)
+			ntfs_error(icx->idx_ni->vol->sb, "Failed to truncate INDEX_ROOT");
+	}
 
 	return ret;
 }
-- 
2.43.0

[PATCH v4 6/6] ntfs: validate resident index root values on lookup

[DaeMyung Kang]

Resident $INDEX_ROOT values carry index header fields that callers
consume after lookup. Some callers already validate parts of the layout
before walking entries, but those checks are scattered and do not cover
all root header invariants, such as entries_offset alignment and lower
bound, index_length, and allocated_size consistency.

Now that ntfs_ir_truncate() updates index.allocated_size before
shrinking the resident value, lookup-time validation can cover these
header invariants without tripping over the driver shrink path.

Add $INDEX_ROOT to the minimum resident value size table and validate the
resident index header fields before returning the attribute from lookup.
Require 8-byte aligned index header fields, a sane entries_offset, an
index_length within allocated_size, allocated_size within the resident
value, and enough entry space for at least an index entry header.

Signed-off-by: DaeMyung Kang <charsyam@gmail.com>
---
 fs/ntfs/attrib.c | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/fs/ntfs/attrib.c b/fs/ntfs/attrib.c
index 7e293b85ad19..437bf400a13a 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:
@@ -615,6 +617,31 @@ static bool ntfs_volume_name_attr_value_is_valid(const u32 value_length)
 	return value_length <= NTFS_MAX_LABEL_LEN * sizeof(__le16);
 }
 
+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;
+
+	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;
@@ -688,6 +715,10 @@ static bool ntfs_attr_value_is_valid(struct ntfs_volume *vol,
 		if (!ntfs_volume_name_attr_value_is_valid(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;
 
-- 
2.43.0

Re: [PATCH v4 6/6] ntfs: validate resident index root values on lookup

[Namjae Jeon]

On Sat, May 30, 2026 at 11:35 PM DaeMyung Kang <charsyam@gmail.com> wrote:
>
> Resident $INDEX_ROOT values carry index header fields that callers
> consume after lookup. Some callers already validate parts of the layout
> before walking entries, but those checks are scattered and do not cover
> all root header invariants, such as entries_offset alignment and lower
> bound, index_length, and allocated_size consistency.
>
> Now that ntfs_ir_truncate() updates index.allocated_size before
> shrinking the resident value, lookup-time validation can cover these
> header invariants without tripping over the driver shrink path.
>
> Add $INDEX_ROOT to the minimum resident value size table and validate the
> resident index header fields before returning the attribute from lookup.
> Require 8-byte aligned index header fields, a sane entries_offset, an
> index_length within allocated_size, allocated_size within the resident
> value, and enough entry space for at least an index entry header.
>
> Signed-off-by: DaeMyung Kang <charsyam@gmail.com>
When this patch is applied, the generic/013 test fails randomly. The
failure can be reproduced by running xfstests generic/013 multiple
times. Additionally, ntfsprogs-plus must be installed to run fsck
checks.
Thanks.

Re: [PATCH v4 6/6] ntfs: validate resident index root values on lookup

[CharSyam]

Hi, Namjae.
I will check it soon. Do you mean 6/6 patch or all of them?

Thanks.
DaeMyung.

2026년 6월 1일 (월) 오후 1:37, Namjae Jeon <linkinjeon@kernel.org>님이 작성:
>
> On Sat, May 30, 2026 at 11:35 PM DaeMyung Kang <charsyam@gmail.com> wrote:
> >
> > Resident $INDEX_ROOT values carry index header fields that callers
> > consume after lookup. Some callers already validate parts of the layout
> > before walking entries, but those checks are scattered and do not cover
> > all root header invariants, such as entries_offset alignment and lower
> > bound, index_length, and allocated_size consistency.
> >
> > Now that ntfs_ir_truncate() updates index.allocated_size before
> > shrinking the resident value, lookup-time validation can cover these
> > header invariants without tripping over the driver shrink path.
> >
> > Add $INDEX_ROOT to the minimum resident value size table and validate the
> > resident index header fields before returning the attribute from lookup.
> > Require 8-byte aligned index header fields, a sane entries_offset, an
> > index_length within allocated_size, allocated_size within the resident
> > value, and enough entry space for at least an index entry header.
> >
> > Signed-off-by: DaeMyung Kang <charsyam@gmail.com>
> When this patch is applied, the generic/013 test fails randomly. The
> failure can be reproduced by running xfstests generic/013 multiple
> times. Additionally, ntfsprogs-plus must be installed to run fsck
> checks.
> Thanks.

Re: [PATCH v4 6/6] ntfs: validate resident index root values on lookup

[Namjae Jeon]

On Mon, Jun 1, 2026 at 4:51 PM CharSyam <charsyam@gmail.com> wrote:
>
> Hi, Namjae.
> I will check it soon. Do you mean 6/6 patch or all of them?
It seems that the generic/013 failure started happening after applying
the 6th patch.
Thanks!
>
> Thanks.
> DaeMyung.
>
> 2026년 6월 1일 (월) 오후 1:37, Namjae Jeon <linkinjeon@kernel.org>님이 작성:
> >
> > On Sat, May 30, 2026 at 11:35 PM DaeMyung Kang <charsyam@gmail.com> wrote:
> > >
> > > Resident $INDEX_ROOT values carry index header fields that callers
> > > consume after lookup. Some callers already validate parts of the layout
> > > before walking entries, but those checks are scattered and do not cover
> > > all root header invariants, such as entries_offset alignment and lower
> > > bound, index_length, and allocated_size consistency.
> > >
> > > Now that ntfs_ir_truncate() updates index.allocated_size before
> > > shrinking the resident value, lookup-time validation can cover these
> > > header invariants without tripping over the driver shrink path.
> > >
> > > Add $INDEX_ROOT to the minimum resident value size table and validate the
> > > resident index header fields before returning the attribute from lookup.
> > > Require 8-byte aligned index header fields, a sane entries_offset, an
> > > index_length within allocated_size, allocated_size within the resident
> > > value, and enough entry space for at least an index entry header.
> > >
> > > Signed-off-by: DaeMyung Kang <charsyam@gmail.com>
> > When this patch is applied, the generic/013 test fails randomly. The
> > failure can be reproduced by running xfstests generic/013 multiple
> > times. Additionally, ntfsprogs-plus must be installed to run fsck
> > checks.
> > Thanks.

Re: [PATCH v4 0/6] ntfs: validate attribute values on lookup

[Namjae Jeon]

On Sat, May 30, 2026 at 11:35 PM DaeMyung Kang <charsyam@gmail.com> wrote:
>
> This v4 supersedes the previous single-patch v3. Per review, it expands
> the lookup-time attribute value validation into a series so $VOLUME_NAME
> and $INDEX_ROOT can be validated safely.
>
> The first patch keeps the original $FILE_NAME corruption fix and moves
> the duplicated non-resident mapping-pairs metadata checks into the common
> attribute value validator. It also keeps resident value matching in the
> external attribute lookup path on the actual value length and fixes the
> matching attrlist duplicate check for resident attributes. The next
> patches prepare the volume label write and mount read paths to cope with
> a rejected $VOLUME_NAME, add $VOLUME_NAME validation, fix the
> ntfs_ir_truncate() shrink ordering issue, and then enable $INDEX_ROOT
> validation.
>
> Changes since v3:
> - Expand the single patch into a 6-patch series as requested.
> - Keep corruption messages using numeric attribute types and drop the
>   attribute type name helper.
> - Move non-resident mapping-pairs metadata validation into the shared
>   ntfs_attr_value_is_valid() helper.
> - Preserve resident @val matching in ntfs_external_attr_find() using the
>   actual resident value length, and avoid reading the non-resident
>   lowest_vcn union member when checking resident attrlist duplicates.
> - Make ntfs_write_volume_label() add a replacement only after successful
>   removal or after -ENOENT, and propagate other lookup/removal errors.
> - Reinitialize the mount-time search context before the
>   $VOLUME_INFORMATION lookup, since a rejected $VOLUME_NAME leaves the
>   context in an undefined state.
> - Add $VOLUME_NAME-specific validation, and reject non-resident
>   $VOLUME_NAME records like non-resident $FILE_NAME.
> - Include the ntfs_ir_truncate() shrink ordering fix.
> - Add $INDEX_ROOT-specific validation on top of the shrink fix.
>
> DaeMyung Kang (6):
>   ntfs: validate attribute values on lookup
>   ntfs: do not replace volume name after lookup errors
>   ntfs: reinit search context before volume information lookup
>   ntfs: validate resident volume name values on lookup
>   ntfs: update index root allocated size before shrink
>   ntfs: validate resident index root values on lookup
I have applied patches 0001 ~ 0004 first, as we are continuously
receiving similar patches. I skipped patch 0005 for now since it seems
like a workaround, and as you know, 0006 patch couldn't be applied due
to xfstests failures.
Thanks!

Re: [PATCH v4 6/6] ntfs: validate resident index root values on lookup

[Namjae Jeon]

> +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;
> +
> +       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 ||
If ntfs driver does not use the allocated_size field of index root,
how about removing the allocated_size checks ?
we can also skip the shrink/grow ordering adjustment in
ntfs_ir_truncate() in your 0005 patch.

> +           index_length - entries_offset < sizeof(struct index_entry_header))
> +               return false;
> +
> +       return true;
> +}
> +

Re: [PATCH v4 0/6] ntfs: validate attribute values on lookup

[CharSyam]

Hi Namjae,

Thanks for applying patches 0001 through 0004.

Skipping 0005/0006 for now makes sense. After re-checking the
generic/013 failure, I found that v4 only fixed one side of the
$INDEX_ROOT resize ordering. Patch 0005 handled the shrink side in
ntfs_ir_truncate(), but v4 missed the grow side in ntfs_ir_reparent():
the code could publish a larger index_length/allocated_size before the
resident value was grown. If that resize then hit -ENOSPC, the recovery
path could re-lookup a transiently inconsistent $INDEX_ROOT.

So 0005 was not intended as a workaround, but it was incomplete as a
prerequisite for the $INDEX_ROOT validator. I will send the remaining
$INDEX_ROOT work as a small v5 follow-up series on top of the tree with
0001-0004 applied.

For the allocated_size question: the driver currently consumes
index.allocated_size as the usable capacity of the shared index_header,
including resident $INDEX_ROOTs. ntfs_ie_add() uses it to decide whether
an insertion can be done in place or whether the root has to grow, and
ntfs_ie_insert() does not re-check the resident value boundary. So I
think the validator should still reject allocated_size values that extend
past the resident index area.

The extent-inode lifetime race I found while testing is independent, so
I will send that separately instead of mixing it into this series.

Thanks,
DaeMyung

2026년 6월 6일 (토) 오전 12:30, Namjae Jeon <linkinjeon@kernel.org>님이 작성:
>
> On Sat, May 30, 2026 at 11:35 PM DaeMyung Kang <charsyam@gmail.com> wrote:
> >
> > This v4 supersedes the previous single-patch v3. Per review, it expands
> > the lookup-time attribute value validation into a series so $VOLUME_NAME
> > and $INDEX_ROOT can be validated safely.
> >
> > The first patch keeps the original $FILE_NAME corruption fix and moves
> > the duplicated non-resident mapping-pairs metadata checks into the common
> > attribute value validator. It also keeps resident value matching in the
> > external attribute lookup path on the actual value length and fixes the
> > matching attrlist duplicate check for resident attributes. The next
> > patches prepare the volume label write and mount read paths to cope with
> > a rejected $VOLUME_NAME, add $VOLUME_NAME validation, fix the
> > ntfs_ir_truncate() shrink ordering issue, and then enable $INDEX_ROOT
> > validation.
> >
> > Changes since v3:
> > - Expand the single patch into a 6-patch series as requested.
> > - Keep corruption messages using numeric attribute types and drop the
> >   attribute type name helper.
> > - Move non-resident mapping-pairs metadata validation into the shared
> >   ntfs_attr_value_is_valid() helper.
> > - Preserve resident @val matching in ntfs_external_attr_find() using the
> >   actual resident value length, and avoid reading the non-resident
> >   lowest_vcn union member when checking resident attrlist duplicates.
> > - Make ntfs_write_volume_label() add a replacement only after successful
> >   removal or after -ENOENT, and propagate other lookup/removal errors.
> > - Reinitialize the mount-time search context before the
> >   $VOLUME_INFORMATION lookup, since a rejected $VOLUME_NAME leaves the
> >   context in an undefined state.
> > - Add $VOLUME_NAME-specific validation, and reject non-resident
> >   $VOLUME_NAME records like non-resident $FILE_NAME.
> > - Include the ntfs_ir_truncate() shrink ordering fix.
> > - Add $INDEX_ROOT-specific validation on top of the shrink fix.
> >
> > DaeMyung Kang (6):
> >   ntfs: validate attribute values on lookup
> >   ntfs: do not replace volume name after lookup errors
> >   ntfs: reinit search context before volume information lookup
> >   ntfs: validate resident volume name values on lookup
> >   ntfs: update index root allocated size before shrink
> >   ntfs: validate resident index root values on lookup
> I have applied patches 0001 ~ 0004 first, as we are continuously
> receiving similar patches. I skipped patch 0005 for now since it seems
> like a workaround, and as you know, 0006 patch couldn't be applied due
> to xfstests failures.
> Thanks!