mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v6 0/4] ntfs: finish resident attribute lookup validation
@ 2026-06-08 15:49 DaeMyung Kang
  2026-06-08 15:49 ` [PATCH v6 1/4] ntfs: reject non-resident records for resident-only attributes DaeMyung Kang
                   ` (4 more replies)
  0 siblings, 5 replies; 10+ messages in thread
From: DaeMyung Kang @ 2026-06-08 15:49 UTC (permalink / raw)
  To: Namjae Jeon; +Cc: Hyunchul Lee, linux-fsdevel, linux-kernel, DaeMyung Kang

Hi,

this is v6 of the remaining NTFS resident attribute lookup validation
fixes, rebased onto the current ntfs-next branch from git.kernel.org.

v5 was generated against the temporary linux-ntfs ntfs-next test branch,
where the earlier lookup validation patches and the initial resident
$INDEX_ROOT validator were already present. That made the final
allocated_size-only patch depend on context that is not present in the
official ntfs-next branch.

This version is generated on top of:

  git://git.kernel.org/pub/scm/linux/kernel/git/linkinjeon/ntfs.git
  branch ntfs-next
  base: <b3f6cd1d54aa>

The current official ntfs-next branch already contains the earlier shared
attribute-value validator, the volume-name error handling fix, the volume
information search-context reinit fix, and the resident $VOLUME_NAME
validator. This resend therefore contains only the remaining official-base
delta:

- reject non-resident records for all resident-only attribute types;
- fix the $INDEX_ROOT grow and shrink ordering so header capacity never
  exceeds the resident value visible to lookup-time validation;
- add the full resident $INDEX_ROOT lookup validator, including
  allocated_size checks.

Changes since v5:
- Rebased onto the official git.kernel.org ntfs-next branch instead of
  the temporary out-of-tree test branch, so the series no longer depends
  on patches that only existed there.
- Patch 4 is now a complete resident $INDEX_ROOT validator. In v5 it was
  an incremental allocated_size-only change on top of the out-of-tree
  validator; the resulting validator function is unchanged, but it now
  stands on its own against ntfs-next.
- No functional changes to the other three patches.

Testing:
- Applied this 4-patch series on top of the ntfs-next base above with
  git am; git diff --check is clean and scripts/checkpatch.pl --strict
  passes on all four patches.
- Built a KASAN kernel (CONFIG_KASAN=y, CONFIG_NTFS_FS=y) and ran it
  under QEMU against fresh 512 MiB NTFS images: mounted each read-write,
  created 12 directories and 2160 files, then renamed, removed, cleaned
  up and unmounted. Repeated three times; no KASAN report, oops or
  panic, and ntfsfix -n reported each image clean after every run.

DaeMyung Kang (4):
  ntfs: reject non-resident records for resident-only attributes
  ntfs: grow index root value before reparent header update
  ntfs: update index root allocated size before shrink
  ntfs: validate resident index root values on lookup

 fs/ntfs/attrib.c | 49 +++++++++++++++++++++++-
 fs/ntfs/index.c  | 96 +++++++++++++++++++++++++++++++-----------------
 2 files changed, 111 insertions(+), 34 deletions(-)


base-commit: b3f6cd1d54aa279cc4f47aa27939ebe517a2c390
-- 
2.43.0

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

* [PATCH v6 1/4] ntfs: reject non-resident records for resident-only attributes
  2026-06-08 15:49 [PATCH v6 0/4] ntfs: finish resident attribute lookup validation DaeMyung Kang
@ 2026-06-08 15:49 ` DaeMyung Kang
  2026-06-08 23:56   ` Hyunchul Lee
  2026-06-08 15:49 ` [PATCH v6 2/4] ntfs: grow index root value before reparent header update DaeMyung Kang
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 10+ messages in thread
From: DaeMyung Kang @ 2026-06-08 15:49 UTC (permalink / raw)
  To: Namjae Jeon; +Cc: Hyunchul Lee, linux-fsdevel, linux-kernel, DaeMyung Kang

The shared lookup-time attribute validator rejects non-resident
$FILE_NAME and $VOLUME_NAME records because their formats require
resident values and callers handle returned records as resident
attributes. Other resident-only attribute types still pass through the
generic non-resident mapping-pairs checks.

That leaves real resident/non-resident union confusion paths. Inode load
looks up $STANDARD_INFORMATION and then reads data.resident.value_offset
without checking a->non_resident. ntfs_inode_sync_standard_information()
does the same when updating the standard information value.
ntfs_write_volume_flags() also looks up $VOLUME_INFORMATION and reads
data.resident.value_offset directly. $INDEX_ROOT callers in dir.c and
index.c depend on the same lookup contract before consuming the resident
index root value.

Reject non-resident records for all resident-only attribute types in the
shared validator. Keep the existing $FILE_NAME and $VOLUME_NAME behavior,
but factor it through a helper and extend it to
$STANDARD_INFORMATION, $OBJECT_ID, $VOLUME_INFORMATION, $INDEX_ROOT, and
$EA_INFORMATION. For $OBJECT_ID and $EA_INFORMATION this is contract
hardening for resident-only formats; this patch only rejects the
non-resident form and does not add new resident value validation for
those types.

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

diff --git a/fs/ntfs/attrib.c b/fs/ntfs/attrib.c
index 5e1ad1cd0118..1254fbe7666e 100644
--- a/fs/ntfs/attrib.c
+++ b/fs/ntfs/attrib.c
@@ -596,6 +596,22 @@ static u32 ntfs_resident_attr_min_value_length(const __le32 type)
 	}
 }
 
+static bool ntfs_attr_type_is_resident_only(const __le32 type)
+{
+	switch (type) {
+	case AT_STANDARD_INFORMATION:
+	case AT_FILE_NAME:
+	case AT_OBJECT_ID:
+	case AT_VOLUME_NAME:
+	case AT_VOLUME_INFORMATION:
+	case AT_INDEX_ROOT:
+	case AT_EA_INFORMATION:
+		return true;
+	default:
+		return false;
+	}
+}
+
 static bool ntfs_file_name_attr_value_is_valid(const u8 *value, const u32 value_length)
 {
 	const struct file_name_attr *fn;
@@ -666,7 +682,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 || a->type == AT_VOLUME_NAME)
+		if (ntfs_attr_type_is_resident_only(a->type))
 			goto corrupt;
 		if (!ntfs_non_resident_attr_value_is_valid(a))
 			goto corrupt;
-- 
2.43.0


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

* [PATCH v6 2/4] ntfs: grow index root value before reparent header update
  2026-06-08 15:49 [PATCH v6 0/4] ntfs: finish resident attribute lookup validation DaeMyung Kang
  2026-06-08 15:49 ` [PATCH v6 1/4] ntfs: reject non-resident records for resident-only attributes DaeMyung Kang
@ 2026-06-08 15:49 ` DaeMyung Kang
  2026-06-08 23:56   ` Hyunchul Lee
  2026-06-08 15:49 ` [PATCH v6 3/4] ntfs: update index root allocated size before shrink DaeMyung Kang
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 10+ messages in thread
From: DaeMyung Kang @ 2026-06-08 15:49 UTC (permalink / raw)
  To: Namjae Jeon; +Cc: Hyunchul Lee, linux-fsdevel, linux-kernel, DaeMyung Kang

ntfs_ir_reparent() moves the resident index root entries into an index
block and leaves a small root stub containing the child VCN. That root
stub can be larger than the existing resident value. For example, an
empty root with value_length 48 has an index area of 32 bytes, while the
large-index root stub needs index_length and allocated_size of 40 bytes.

The current code publishes the larger index.index_length and
index.allocated_size before resizing the resident value. If the resize
returns -ENOSPC, the recovery path can call ntfs_inode_add_attrlist(),
which looks attributes up again while the root header says
allocated_size 40 but the resident value still only provides 32 bytes of
index area. Lookup-time $INDEX_ROOT validation then correctly rejects
that transient layout as corrupt.

This reproduces as a generic/013 failure under qemu. In the failing run,
the transient root had value_len=48, index_size=32, index_length=40, and
allocated_size=40, and ntfsprogs-plus ntfsck reported "Corrupt index
root in MFT record 1177".

When the root stub grows, resize the resident value before publishing the
larger root header. If the resize fails, the old root remains valid for
recovery lookups. Keep the existing header-before-resize ordering for
shrink or same-size cases so the resident value never temporarily
exposes an allocated_size beyond its bounds.

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

diff --git a/fs/ntfs/index.c b/fs/ntfs/index.c
index 8371ff4303e7..052d80fddbbc 100644
--- a/fs/ntfs/index.c
+++ b/fs/ntfs/index.c
@@ -1240,6 +1240,8 @@ static int ntfs_ir_reparent(struct ntfs_index_context *icx)
 	struct index_entry *ie;
 	struct index_block *ib = NULL;
 	s64 new_ib_vcn;
+	u32 index_length;
+	u32 old_value_length;
 	int ix_root_size;
 	int ret = 0;
 
@@ -1287,6 +1289,21 @@ static int ntfs_ir_reparent(struct ntfs_index_context *icx)
 		goto clear_bmp;
 	}
 
+	old_value_length = le32_to_cpu(ctx->attr->data.resident.value_length);
+	index_length = le32_to_cpu(ir->index.entries_offset) +
+		sizeof(struct index_entry_header) + sizeof(s64);
+	ix_root_size = offsetof(struct index_root, index) + index_length;
+	/* Grow the resident value before publishing the larger root header. */
+	if (ix_root_size > old_value_length) {
+		ret = ntfs_resident_attr_value_resize(ctx->mrec, ctx->attr, ix_root_size);
+		if (ret)
+			goto resize_failed;
+
+		icx->idx_ni->data_size = ix_root_size;
+		icx->idx_ni->initialized_size = ix_root_size;
+		icx->idx_ni->allocated_size = (ix_root_size + 7) & ~7;
+	}
+
 	ntfs_ir_nill(ir);
 
 	ie = ntfs_ie_get_first(&ir->index);
@@ -1295,48 +1312,49 @@ static int ntfs_ir_reparent(struct ntfs_index_context *icx)
 
 	ir->index.flags = LARGE_INDEX;
 	NInoSetIndexAllocPresent(icx->idx_ni);
-	ir->index.index_length = cpu_to_le32(le32_to_cpu(ir->index.entries_offset) +
-			le16_to_cpu(ie->length));
+	ir->index.index_length = cpu_to_le32(index_length);
 	ir->index.allocated_size = ir->index.index_length;
 
-	ix_root_size = sizeof(struct index_root) - sizeof(struct index_header) +
-		le32_to_cpu(ir->index.allocated_size);
-	ret  = ntfs_resident_attr_value_resize(ctx->mrec, ctx->attr, ix_root_size);
-	if (ret) {
-		/*
-		 * When there is no space to build a non-resident
-		 * index, we may have to move the root to an extent
-		 */
-		if ((ret == -ENOSPC) && (ctx->al_entry || !ntfs_inode_add_attrlist(icx->idx_ni))) {
+	if (ix_root_size <= old_value_length) {
+		ret = ntfs_resident_attr_value_resize(ctx->mrec, ctx->attr, ix_root_size);
+		if (ret)
+			goto resize_failed;
+
+		icx->idx_ni->data_size = ix_root_size;
+		icx->idx_ni->initialized_size = ix_root_size;
+		icx->idx_ni->allocated_size = (ix_root_size + 7) & ~7;
+	}
+	ntfs_ie_set_vcn(ie, new_ib_vcn);
+	goto err_out;
+
+resize_failed:
+	/*
+	 * When there is no space to build a non-resident
+	 * index, we may have to move the root to an extent
+	 */
+	if ((ret == -ENOSPC) && (ctx->al_entry || !ntfs_inode_add_attrlist(icx->idx_ni))) {
+		ntfs_attr_put_search_ctx(ctx);
+		ctx = NULL;
+		ir = ntfs_ir_lookup(icx->idx_ni, icx->name, icx->name_len, &ctx);
+		if (ir && !ntfs_attr_record_move_away(ctx, ix_root_size -
+				le32_to_cpu(ctx->attr->data.resident.value_length))) {
+			if (ntfs_attrlist_update(ctx->base_ntfs_ino ?
+						 ctx->base_ntfs_ino : ctx->ntfs_ino))
+				goto clear_bmp;
 			ntfs_attr_put_search_ctx(ctx);
 			ctx = NULL;
-			ir = ntfs_ir_lookup(icx->idx_ni, icx->name, icx->name_len, &ctx);
-			if (ir && !ntfs_attr_record_move_away(ctx, ix_root_size -
-					le32_to_cpu(ctx->attr->data.resident.value_length))) {
-				if (ntfs_attrlist_update(ctx->base_ntfs_ino ?
-							 ctx->base_ntfs_ino : ctx->ntfs_ino))
-					goto clear_bmp;
-				ntfs_attr_put_search_ctx(ctx);
-				ctx = NULL;
-				goto retry;
-			}
+			goto retry;
 		}
-		goto clear_bmp;
-	} else {
-		icx->idx_ni->data_size = icx->idx_ni->initialized_size = ix_root_size;
-		icx->idx_ni->allocated_size = (ix_root_size  + 7) & ~7;
 	}
-	ntfs_ie_set_vcn(ie, new_ib_vcn);
-
+clear_bmp:
+	ntfs_ibm_clear(icx, new_ib_vcn);
+	goto err_out;
 err_out:
 	kvfree(ib);
 	if (ctx)
 		ntfs_attr_put_search_ctx(ctx);
 out:
 	return ret;
-clear_bmp:
-	ntfs_ibm_clear(icx, new_ib_vcn);
-	goto err_out;
 }
 
 /*
-- 
2.43.0


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

* [PATCH v6 3/4] ntfs: update index root allocated size before shrink
  2026-06-08 15:49 [PATCH v6 0/4] ntfs: finish resident attribute lookup validation DaeMyung Kang
  2026-06-08 15:49 ` [PATCH v6 1/4] ntfs: reject non-resident records for resident-only attributes DaeMyung Kang
  2026-06-08 15:49 ` [PATCH v6 2/4] ntfs: grow index root value before reparent header update DaeMyung Kang
@ 2026-06-08 15:49 ` DaeMyung Kang
  2026-06-08 23:57   ` Hyunchul Lee
  2026-06-08 15:49 ` [PATCH v6 4/4] ntfs: validate resident index root values on lookup DaeMyung Kang
  2026-06-09  9:25 ` [PATCH v6 0/4] ntfs: finish resident attribute lookup validation Namjae Jeon
  4 siblings, 1 reply; 10+ messages in thread
From: DaeMyung Kang @ 2026-06-08 15:49 UTC (permalink / raw)
  To: Namjae Jeon; +Cc: Hyunchul Lee, linux-fsdevel, linux-kernel, 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 052d80fddbbc..c5f2cf75b750 100644
--- a/fs/ntfs/index.c
+++ b/fs/ntfs/index.c
@@ -1365,9 +1365,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.
@@ -1379,9 +1386,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


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

* [PATCH v6 4/4] ntfs: validate resident index root values on lookup
  2026-06-08 15:49 [PATCH v6 0/4] ntfs: finish resident attribute lookup validation DaeMyung Kang
                   ` (2 preceding siblings ...)
  2026-06-08 15:49 ` [PATCH v6 3/4] ntfs: update index root allocated size before shrink DaeMyung Kang
@ 2026-06-08 15:49 ` DaeMyung Kang
  2026-06-08 23:58   ` Hyunchul Lee
  2026-06-09  9:25 ` [PATCH v6 0/4] ntfs: finish resident attribute lookup validation Namjae Jeon
  4 siblings, 1 reply; 10+ messages in thread
From: DaeMyung Kang @ 2026-06-08 15:49 UTC (permalink / raw)
  To: Namjae Jeon; +Cc: Hyunchul Lee, linux-fsdevel, linux-kernel, 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.

The resident root resize paths now keep these header fields consistent
while the value size changes: ntfs_ir_truncate() lowers
index.allocated_size before shrinking the resident value, and
ntfs_ir_reparent() grows the resident value before publishing a larger
root header. Lookup-time validation can therefore cover these invariants
without tripping over the driver's own resize paths.

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.

The shared validator already rejects non-resident records for
resident-only attribute types, including $INDEX_ROOT.

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 1254fbe7666e..8f10688a17df 100644
--- a/fs/ntfs/attrib.c
+++ b/fs/ntfs/attrib.c
@@ -589,6 +589,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:
@@ -632,6 +634,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;
@@ -705,6 +732,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


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

* Re: [PATCH v6 1/4] ntfs: reject non-resident records for resident-only attributes
  2026-06-08 15:49 ` [PATCH v6 1/4] ntfs: reject non-resident records for resident-only attributes DaeMyung Kang
@ 2026-06-08 23:56   ` Hyunchul Lee
  0 siblings, 0 replies; 10+ messages in thread
From: Hyunchul Lee @ 2026-06-08 23:56 UTC (permalink / raw)
  To: DaeMyung Kang; +Cc: Namjae Jeon, linux-fsdevel, linux-kernel

2026년 6월 9일 (화) 오전 12:49, DaeMyung Kang <charsyam@gmail.com>님이 작성:
>
> The shared lookup-time attribute validator rejects non-resident
> $FILE_NAME and $VOLUME_NAME records because their formats require
> resident values and callers handle returned records as resident
> attributes. Other resident-only attribute types still pass through the
> generic non-resident mapping-pairs checks.
>
> That leaves real resident/non-resident union confusion paths. Inode load
> looks up $STANDARD_INFORMATION and then reads data.resident.value_offset
> without checking a->non_resident. ntfs_inode_sync_standard_information()
> does the same when updating the standard information value.
> ntfs_write_volume_flags() also looks up $VOLUME_INFORMATION and reads
> data.resident.value_offset directly. $INDEX_ROOT callers in dir.c and
> index.c depend on the same lookup contract before consuming the resident
> index root value.
>
> Reject non-resident records for all resident-only attribute types in the
> shared validator. Keep the existing $FILE_NAME and $VOLUME_NAME behavior,
> but factor it through a helper and extend it to
> $STANDARD_INFORMATION, $OBJECT_ID, $VOLUME_INFORMATION, $INDEX_ROOT, and
> $EA_INFORMATION. For $OBJECT_ID and $EA_INFORMATION this is contract
> hardening for resident-only formats; this patch only rejects the
> non-resident form and does not add new resident value validation for
> those types.
>
> Signed-off-by: DaeMyung Kang <charsyam@gmail.com>

Looks good to me.

Reviewed-by: Hyunchul Lee <hyc.lee@gmail.com>

> ---
>  fs/ntfs/attrib.c | 18 +++++++++++++++++-
>  1 file changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/fs/ntfs/attrib.c b/fs/ntfs/attrib.c
> index 5e1ad1cd0118..1254fbe7666e 100644
> --- a/fs/ntfs/attrib.c
> +++ b/fs/ntfs/attrib.c
> @@ -596,6 +596,22 @@ static u32 ntfs_resident_attr_min_value_length(const __le32 type)
>         }
>  }
>
> +static bool ntfs_attr_type_is_resident_only(const __le32 type)
> +{
> +       switch (type) {
> +       case AT_STANDARD_INFORMATION:
> +       case AT_FILE_NAME:
> +       case AT_OBJECT_ID:
> +       case AT_VOLUME_NAME:
> +       case AT_VOLUME_INFORMATION:
> +       case AT_INDEX_ROOT:
> +       case AT_EA_INFORMATION:
> +               return true;
> +       default:
> +               return false;
> +       }
> +}
> +
>  static bool ntfs_file_name_attr_value_is_valid(const u8 *value, const u32 value_length)
>  {
>         const struct file_name_attr *fn;
> @@ -666,7 +682,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 || a->type == AT_VOLUME_NAME)
> +               if (ntfs_attr_type_is_resident_only(a->type))
>                         goto corrupt;
>                 if (!ntfs_non_resident_attr_value_is_valid(a))
>                         goto corrupt;
> --
> 2.43.0
>


-- 
Thanks,
Hyunchul

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

* Re: [PATCH v6 2/4] ntfs: grow index root value before reparent header update
  2026-06-08 15:49 ` [PATCH v6 2/4] ntfs: grow index root value before reparent header update DaeMyung Kang
@ 2026-06-08 23:56   ` Hyunchul Lee
  0 siblings, 0 replies; 10+ messages in thread
From: Hyunchul Lee @ 2026-06-08 23:56 UTC (permalink / raw)
  To: DaeMyung Kang; +Cc: Namjae Jeon, linux-fsdevel, linux-kernel

2026년 6월 9일 (화) 오전 12:49, DaeMyung Kang <charsyam@gmail.com>님이 작성:
>
> ntfs_ir_reparent() moves the resident index root entries into an index
> block and leaves a small root stub containing the child VCN. That root
> stub can be larger than the existing resident value. For example, an
> empty root with value_length 48 has an index area of 32 bytes, while the
> large-index root stub needs index_length and allocated_size of 40 bytes.
>
> The current code publishes the larger index.index_length and
> index.allocated_size before resizing the resident value. If the resize
> returns -ENOSPC, the recovery path can call ntfs_inode_add_attrlist(),
> which looks attributes up again while the root header says
> allocated_size 40 but the resident value still only provides 32 bytes of
> index area. Lookup-time $INDEX_ROOT validation then correctly rejects
> that transient layout as corrupt.
>
> This reproduces as a generic/013 failure under qemu. In the failing run,
> the transient root had value_len=48, index_size=32, index_length=40, and
> allocated_size=40, and ntfsprogs-plus ntfsck reported "Corrupt index
> root in MFT record 1177".
>
> When the root stub grows, resize the resident value before publishing the
> larger root header. If the resize fails, the old root remains valid for
> recovery lookups. Keep the existing header-before-resize ordering for
> shrink or same-size cases so the resident value never temporarily
> exposes an allocated_size beyond its bounds.
>
> Signed-off-by: DaeMyung Kang <charsyam@gmail.com>

Looks good to me.

Reviewed-by: Hyunchul Lee <hyc.lee@gmail.com>

> ---
>  fs/ntfs/index.c | 78 ++++++++++++++++++++++++++++++-------------------
>  1 file changed, 48 insertions(+), 30 deletions(-)
>
> diff --git a/fs/ntfs/index.c b/fs/ntfs/index.c
> index 8371ff4303e7..052d80fddbbc 100644
> --- a/fs/ntfs/index.c
> +++ b/fs/ntfs/index.c
> @@ -1240,6 +1240,8 @@ static int ntfs_ir_reparent(struct ntfs_index_context *icx)
>         struct index_entry *ie;
>         struct index_block *ib = NULL;
>         s64 new_ib_vcn;
> +       u32 index_length;
> +       u32 old_value_length;
>         int ix_root_size;
>         int ret = 0;
>
> @@ -1287,6 +1289,21 @@ static int ntfs_ir_reparent(struct ntfs_index_context *icx)
>                 goto clear_bmp;
>         }
>
> +       old_value_length = le32_to_cpu(ctx->attr->data.resident.value_length);
> +       index_length = le32_to_cpu(ir->index.entries_offset) +
> +               sizeof(struct index_entry_header) + sizeof(s64);
> +       ix_root_size = offsetof(struct index_root, index) + index_length;
> +       /* Grow the resident value before publishing the larger root header. */
> +       if (ix_root_size > old_value_length) {
> +               ret = ntfs_resident_attr_value_resize(ctx->mrec, ctx->attr, ix_root_size);
> +               if (ret)
> +                       goto resize_failed;
> +
> +               icx->idx_ni->data_size = ix_root_size;
> +               icx->idx_ni->initialized_size = ix_root_size;
> +               icx->idx_ni->allocated_size = (ix_root_size + 7) & ~7;
> +       }
> +
>         ntfs_ir_nill(ir);
>
>         ie = ntfs_ie_get_first(&ir->index);
> @@ -1295,48 +1312,49 @@ static int ntfs_ir_reparent(struct ntfs_index_context *icx)
>
>         ir->index.flags = LARGE_INDEX;
>         NInoSetIndexAllocPresent(icx->idx_ni);
> -       ir->index.index_length = cpu_to_le32(le32_to_cpu(ir->index.entries_offset) +
> -                       le16_to_cpu(ie->length));
> +       ir->index.index_length = cpu_to_le32(index_length);
>         ir->index.allocated_size = ir->index.index_length;
>
> -       ix_root_size = sizeof(struct index_root) - sizeof(struct index_header) +
> -               le32_to_cpu(ir->index.allocated_size);
> -       ret  = ntfs_resident_attr_value_resize(ctx->mrec, ctx->attr, ix_root_size);
> -       if (ret) {
> -               /*
> -                * When there is no space to build a non-resident
> -                * index, we may have to move the root to an extent
> -                */
> -               if ((ret == -ENOSPC) && (ctx->al_entry || !ntfs_inode_add_attrlist(icx->idx_ni))) {
> +       if (ix_root_size <= old_value_length) {
> +               ret = ntfs_resident_attr_value_resize(ctx->mrec, ctx->attr, ix_root_size);
> +               if (ret)
> +                       goto resize_failed;
> +
> +               icx->idx_ni->data_size = ix_root_size;
> +               icx->idx_ni->initialized_size = ix_root_size;
> +               icx->idx_ni->allocated_size = (ix_root_size + 7) & ~7;
> +       }
> +       ntfs_ie_set_vcn(ie, new_ib_vcn);
> +       goto err_out;
> +
> +resize_failed:
> +       /*
> +        * When there is no space to build a non-resident
> +        * index, we may have to move the root to an extent
> +        */
> +       if ((ret == -ENOSPC) && (ctx->al_entry || !ntfs_inode_add_attrlist(icx->idx_ni))) {
> +               ntfs_attr_put_search_ctx(ctx);
> +               ctx = NULL;
> +               ir = ntfs_ir_lookup(icx->idx_ni, icx->name, icx->name_len, &ctx);
> +               if (ir && !ntfs_attr_record_move_away(ctx, ix_root_size -
> +                               le32_to_cpu(ctx->attr->data.resident.value_length))) {
> +                       if (ntfs_attrlist_update(ctx->base_ntfs_ino ?
> +                                                ctx->base_ntfs_ino : ctx->ntfs_ino))
> +                               goto clear_bmp;
>                         ntfs_attr_put_search_ctx(ctx);
>                         ctx = NULL;
> -                       ir = ntfs_ir_lookup(icx->idx_ni, icx->name, icx->name_len, &ctx);
> -                       if (ir && !ntfs_attr_record_move_away(ctx, ix_root_size -
> -                                       le32_to_cpu(ctx->attr->data.resident.value_length))) {
> -                               if (ntfs_attrlist_update(ctx->base_ntfs_ino ?
> -                                                        ctx->base_ntfs_ino : ctx->ntfs_ino))
> -                                       goto clear_bmp;
> -                               ntfs_attr_put_search_ctx(ctx);
> -                               ctx = NULL;
> -                               goto retry;
> -                       }
> +                       goto retry;
>                 }
> -               goto clear_bmp;
> -       } else {
> -               icx->idx_ni->data_size = icx->idx_ni->initialized_size = ix_root_size;
> -               icx->idx_ni->allocated_size = (ix_root_size  + 7) & ~7;
>         }
> -       ntfs_ie_set_vcn(ie, new_ib_vcn);
> -
> +clear_bmp:
> +       ntfs_ibm_clear(icx, new_ib_vcn);
> +       goto err_out;
>  err_out:
>         kvfree(ib);
>         if (ctx)
>                 ntfs_attr_put_search_ctx(ctx);
>  out:
>         return ret;
> -clear_bmp:
> -       ntfs_ibm_clear(icx, new_ib_vcn);
> -       goto err_out;
>  }
>
>  /*
> --
> 2.43.0
>


-- 
Thanks,
Hyunchul

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

* Re: [PATCH v6 3/4] ntfs: update index root allocated size before shrink
  2026-06-08 15:49 ` [PATCH v6 3/4] ntfs: update index root allocated size before shrink DaeMyung Kang
@ 2026-06-08 23:57   ` Hyunchul Lee
  0 siblings, 0 replies; 10+ messages in thread
From: Hyunchul Lee @ 2026-06-08 23:57 UTC (permalink / raw)
  To: DaeMyung Kang; +Cc: Namjae Jeon, linux-fsdevel, linux-kernel

2026년 6월 9일 (화) 오전 12:49, DaeMyung Kang <charsyam@gmail.com>님이 작성:
>
> 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>

Looks good to me.

Reviewed-by: Hyunchul Lee <hyc.lee@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 052d80fddbbc..c5f2cf75b750 100644
> --- a/fs/ntfs/index.c
> +++ b/fs/ntfs/index.c
> @@ -1365,9 +1365,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.
> @@ -1379,9 +1386,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
>


-- 
Thanks,
Hyunchul

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

* Re: [PATCH v6 4/4] ntfs: validate resident index root values on lookup
  2026-06-08 15:49 ` [PATCH v6 4/4] ntfs: validate resident index root values on lookup DaeMyung Kang
@ 2026-06-08 23:58   ` Hyunchul Lee
  0 siblings, 0 replies; 10+ messages in thread
From: Hyunchul Lee @ 2026-06-08 23:58 UTC (permalink / raw)
  To: DaeMyung Kang; +Cc: Namjae Jeon, linux-fsdevel, linux-kernel

2026년 6월 9일 (화) 오전 12:49, DaeMyung Kang <charsyam@gmail.com>님이 작성:
>
> 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.
>
> The resident root resize paths now keep these header fields consistent
> while the value size changes: ntfs_ir_truncate() lowers
> index.allocated_size before shrinking the resident value, and
> ntfs_ir_reparent() grows the resident value before publishing a larger
> root header. Lookup-time validation can therefore cover these invariants
> without tripping over the driver's own resize paths.
>
> 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.
>
> The shared validator already rejects non-resident records for
> resident-only attribute types, including $INDEX_ROOT.
>
> Signed-off-by: DaeMyung Kang <charsyam@gmail.com>

Looks good to me.

Reviewed-by: Hyunchul Lee <hyc.lee@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 1254fbe7666e..8f10688a17df 100644
> --- a/fs/ntfs/attrib.c
> +++ b/fs/ntfs/attrib.c
> @@ -589,6 +589,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:
> @@ -632,6 +634,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;
> @@ -705,6 +732,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
>


-- 
Thanks,
Hyunchul

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

* Re: [PATCH v6 0/4] ntfs: finish resident attribute lookup validation
  2026-06-08 15:49 [PATCH v6 0/4] ntfs: finish resident attribute lookup validation DaeMyung Kang
                   ` (3 preceding siblings ...)
  2026-06-08 15:49 ` [PATCH v6 4/4] ntfs: validate resident index root values on lookup DaeMyung Kang
@ 2026-06-09  9:25 ` Namjae Jeon
  4 siblings, 0 replies; 10+ messages in thread
From: Namjae Jeon @ 2026-06-09  9:25 UTC (permalink / raw)
  To: DaeMyung Kang; +Cc: Hyunchul Lee, linux-fsdevel, linux-kernel

On Tue, Jun 9, 2026 at 12:49 AM DaeMyung Kang <charsyam@gmail.com> wrote:
>
> Hi,
>
> this is v6 of the remaining NTFS resident attribute lookup validation
> fixes, rebased onto the current ntfs-next branch from git.kernel.org.
>
> v5 was generated against the temporary linux-ntfs ntfs-next test branch,
> where the earlier lookup validation patches and the initial resident
> $INDEX_ROOT validator were already present. That made the final
> allocated_size-only patch depend on context that is not present in the
> official ntfs-next branch.
>
> This version is generated on top of:
>
>   git://git.kernel.org/pub/scm/linux/kernel/git/linkinjeon/ntfs.git
>   branch ntfs-next
>   base: <b3f6cd1d54aa>
>
> The current official ntfs-next branch already contains the earlier shared
> attribute-value validator, the volume-name error handling fix, the volume
> information search-context reinit fix, and the resident $VOLUME_NAME
> validator. This resend therefore contains only the remaining official-base
> delta:
>
> - reject non-resident records for all resident-only attribute types;
> - fix the $INDEX_ROOT grow and shrink ordering so header capacity never
>   exceeds the resident value visible to lookup-time validation;
> - add the full resident $INDEX_ROOT lookup validator, including
>   allocated_size checks.
>
> Changes since v5:
> - Rebased onto the official git.kernel.org ntfs-next branch instead of
>   the temporary out-of-tree test branch, so the series no longer depends
>   on patches that only existed there.
> - Patch 4 is now a complete resident $INDEX_ROOT validator. In v5 it was
>   an incremental allocated_size-only change on top of the out-of-tree
>   validator; the resulting validator function is unchanged, but it now
>   stands on its own against ntfs-next.
> - No functional changes to the other three patches.
>
> Testing:
> - Applied this 4-patch series on top of the ntfs-next base above with
>   git am; git diff --check is clean and scripts/checkpatch.pl --strict
>   passes on all four patches.
> - Built a KASAN kernel (CONFIG_KASAN=y, CONFIG_NTFS_FS=y) and ran it
>   under QEMU against fresh 512 MiB NTFS images: mounted each read-write,
>   created 12 directories and 2160 files, then renamed, removed, cleaned
>   up and unmounted. Repeated three times; no KASAN report, oops or
>   panic, and ntfsfix -n reported each image clean after every run.
>
> DaeMyung Kang (4):
>   ntfs: reject non-resident records for resident-only attributes
>   ntfs: grow index root value before reparent header update
>   ntfs: update index root allocated size before shrink
>   ntfs: validate resident index root values on lookup
Applied them to #ntfs-next.
Thanks!

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

end of thread, other threads:[~2026-06-09  9:25 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-06-08 15:49 [PATCH v6 0/4] ntfs: finish resident attribute lookup validation DaeMyung Kang
2026-06-08 15:49 ` [PATCH v6 1/4] ntfs: reject non-resident records for resident-only attributes DaeMyung Kang
2026-06-08 23:56   ` Hyunchul Lee
2026-06-08 15:49 ` [PATCH v6 2/4] ntfs: grow index root value before reparent header update DaeMyung Kang
2026-06-08 23:56   ` Hyunchul Lee
2026-06-08 15:49 ` [PATCH v6 3/4] ntfs: update index root allocated size before shrink DaeMyung Kang
2026-06-08 23:57   ` Hyunchul Lee
2026-06-08 15:49 ` [PATCH v6 4/4] ntfs: validate resident index root values on lookup DaeMyung Kang
2026-06-08 23:58   ` Hyunchul Lee
2026-06-09  9:25 ` [PATCH v6 0/4] ntfs: finish resident attribute lookup validation Namjae Jeon

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