[PATCH v2 0/3] ntfs: three fixes for crafted images, found by fuzzing

[PATCH v2 0/3] ntfs: three fixes for crafted images, found by fuzzing

[Wahid Khan]

I fuzz the new fs/ntfs driver with mutated images: an image from mkntfs
populated with ntfs-3g, a few bytes changed, then mount, walk the tree
and write, under KASAN. These three problems are still present in
ntfs-next (e62b384ca). Each patch has its trace and reproduction steps.

1. ntfs: do not read an index block outside $INDEX_ALLOCATION
   A directory without a usable $INDEX_ALLOCATION hangs on its first
   lookup. The task waits for an mft record lock that it already holds.
2. ntfs: open the index allocation before reading or writing an index block
   NULL pointer dereference in ntfs_ib_write() on link().
3. ntfs: do not convert a named $DATA stream when making room in a record
   WARN_ON() in ntfs_attr_make_non_resident(); no damaged volume needed.

All three need an image to be mounted, so a crafted image, not a remote
attacker.

Changes in v2, after an automated review of v1 that I checked by hand:
- 2/3: ntfs_ia_open() also succeeds for a directory without
  $INDEX_ALLOCATION and returns an empty resident inode. Reject an index
  inode that is not non-resident, as 1/3 already does.
- 3/3: v1 opened the named stream and converted it. That would lock the
  stream's folio under the mft record lock, the reverse of the read
  path (ntfs_read_iomap_begin_resident()). v2 skips named streams in
  that loop instead.
- 1/3: unchanged.

v1: https://lore.kernel.org/all/20261106091420.2461683-1-wahidzk0091@gmail.com/


Wahid Khan (3):
  ntfs: do not read an index block outside $INDEX_ALLOCATION
  ntfs: open the index allocation before reading or writing an index
    block
  ntfs: do not convert a named $DATA stream when making room in a record

 fs/ntfs/attrib.c |  6 ++++++
 fs/ntfs/dir.c    | 14 ++++++++++++++
 fs/ntfs/index.c  | 22 ++++++++++++++++++++++
 3 files changed, 42 insertions(+)

-- 
2.53.0

[PATCH v2 1/3] ntfs: do not read an index block outside $INDEX_ALLOCATION

[Wahid Khan]

A directory whose $INDEX_ROOT has a child node but whose
$INDEX_ALLOCATION is missing or damaged gets an empty index inode from
ntfs_read_locked_index_inode(): size 0 and not non-resident.
ntfs_lookup_inode_by_name() still follows the child node and reads a
folio from that inode while it holds the mft record lock of the
directory.

Since the resident iomap read started to take base_ni->mrec_lock, that
read waits for a lock the caller already holds:

  INFO: task init:1 is blocked on a mutex likely owned by task init:1.
   __mutex_lock
   __ntfs_read_iomap_begin
   ntfs_read_iomap_next
   iomap_read_folio
   ntfs_read_folio
   ntfs_lookup_inode_by_name
   ntfs_lookup

The task never returns, and neither does anything else that needs the
directory's mft record.

It takes one corrupted byte on an image from mkntfs: change the first
byte of the type of the $INDEX_ALLOCATION attribute header of a
directory with index blocks from 0xa0 to 0x00. The first lookup in that
directory hangs.

Check that the index inode is non-resident and that the vcn lies inside
it before every index block read, and fail with -EIO otherwise. This
also rejects a negative vcn, which was only checked on later descents.

Fixes: d9e00c457d4a ("ntfs: serialize resident iomap reads with mrec_lock")
Cc: stable@vger.kernel.org # v7.3+
Signed-off-by: Wahid Khan <wahidzk0091@gmail.com>
---
 fs/ntfs/dir.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/fs/ntfs/dir.c b/fs/ntfs/dir.c
index fce10baa2..9fb5bb3d3 100644
--- a/fs/ntfs/dir.c
+++ b/fs/ntfs/dir.c
@@ -308,6 +308,20 @@ u64 ntfs_lookup_inode_by_name(struct ntfs_inode *dir_ni, const __le16 *uname,
 
 	ia_mapping = ia_vi->i_mapping;
 descend_into_child_node:
+	/*
+	 * The child node has to be inside $INDEX_ALLOCATION. A directory that
+	 * lacks the attribute gets an empty index inode that is not
+	 * non-resident, and reading a folio from it locks the mft record that
+	 * is already locked here.
+	 */
+	if (!NInoNonResident(NTFS_I(ia_vi)) || vcn < 0 ||
+	    (u64)vcn >= (u64)i_size_read(ia_vi) >> dir_ni->itype.index.vcn_size_bits) {
+		ntfs_error(sb,
+			"Child node vcn %lld is outside $INDEX_ALLOCATION in directory inode 0x%llx.",
+			vcn, dir_ni->mft_no);
+		err = -EIO;
+		goto err_out;
+	}
 	/*
 	 * Convert vcn to index into the index allocation attribute in units
 	 * of PAGE_SIZE and map the page cache page, reading it from
-- 
2.53.0

[PATCH v2 2/3] ntfs: open the index allocation before reading or writing an index block

[Wahid Khan]

ntfs_ib_write() and ntfs_ib_read() use icx->ia_ni without checking it.
ntfs_ir_reparent() only opens the index allocation through
ntfs_ia_add() when the root is a small index. For a root that is already
marked as a large index it calls ntfs_ib_write() directly.

The lookup does not open the index allocation either when it ends in the
root. If the last entry of the root has no child node, ia_ni is still
NULL when the root has to be moved into an index block, and the write
dereferences it:

  Oops: general protection fault, KASAN: null-ptr-deref in range
        [0x0000000000000128-0x000000000000012f]
  RIP: 0010:ntfs_inode_attr_pwrite+0x52/0x1600
   ntfs_ir_make_space
   ntfs_ie_add
   ntfs_index_add_filename
   __ntfs_link
   ntfs_link
   vfs_link

A single byte of an image from mkntfs is enough. In a directory that has
one empty index block, clear INDEX_ENTRY_NODE (flags 3 -> 2) of the last
entry of the $INDEX_ROOT and link() a file into the directory.

Commit 9365c4a1357e ("ntfs: fix NULL dereference on index allocation open failure")
covers the case where ntfs_ia_open() fails in the lookup. Here it was
never tried. Open the index allocation on demand in both
helpers and fail with -EIO if that does not work.

ntfs_ia_open() also succeeds for a directory that has no
$INDEX_ALLOCATION attribute. The inode it returns is then an empty resident
stub that does not describe an index allocation, so reject that case too.

Fixes: 0a8ac0c1fa0b ("ntfs: update directory operations")
Cc: stable@vger.kernel.org # v7.1+
Signed-off-by: Wahid Khan <wahidzk0091@gmail.com>
---
 fs/ntfs/index.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/fs/ntfs/index.c b/fs/ntfs/index.c
index 322cb371d..74716f916 100644
--- a/fs/ntfs/index.c
+++ b/fs/ntfs/index.c
@@ -99,12 +99,30 @@ static s64 ntfs_ib_pos_to_vcn(struct ntfs_index_context *icx, s64 pos)
 	return pos >> icx->vcn_size_bits;
 }
 
+/* Open the index allocation inode if no earlier step of the operation did. */
+static int ntfs_icx_ia_ensure(struct ntfs_index_context *icx)
+{
+	if (!icx->ia_ni) {
+		icx->ia_ni = ntfs_ia_open(icx, icx->idx_ni);
+		if (!icx->ia_ni)
+			return -EIO;
+	}
+	/* Without $INDEX_ALLOCATION the index inode is an empty resident stub. */
+	if (!NInoNonResident(icx->ia_ni))
+		return -EIO;
+	return 0;
+}
+
 static int ntfs_ib_write(struct ntfs_index_context *icx, struct index_block *ib)
 {
 	s64 ret, vcn = le64_to_cpu(ib->index_block_vcn);
 
 	ntfs_debug("vcn: %lld\n", vcn);
 
+	ret = ntfs_icx_ia_ensure(icx);
+	if (ret)
+		return ret;
+
 	ret = pre_write_mst_fixup((struct ntfs_record *)ib, icx->block_size);
 	if (ret)
 		return -EIO;
@@ -805,6 +823,10 @@ static int ntfs_ib_read(struct ntfs_index_context *icx, s64 vcn, struct index_bl
 
 	ntfs_debug("vcn: %lld\n", vcn);
 
+	ret = ntfs_icx_ia_ensure(icx);
+	if (ret)
+		return ret;
+
 	pos = ntfs_ib_vcn_to_pos(icx, vcn);
 
 	ret = ntfs_inode_attr_pread(VFS_I(icx->ia_ni), pos, icx->block_size, (u8 *)dst);
-- 
2.53.0

[PATCH v2 3/3] ntfs: do not convert a named $DATA stream when making room in a record

[Wahid Khan]

When an attribute has to grow and the mft record is full,
ntfs_resident_attr_resize() walks the other resident attributes and
converts them to non-resident ones. For $DATA it always opens the
unnamed data inode of the base inode, even if the attribute found is a
named stream:

  tvi = ntfs_iget(sb, base_ni->mft_no);
  ntfs_attr_make_non_resident(NTFS_I(tvi), value_len);

value_len is the length of the stream, so it does not match the
attribute that ntfs_attr_make_non_resident() looks up for the unnamed
inode, and the WARN_ON() on entry fires:

  WARNING: fs/ntfs/attrib.c:2115 at ntfs_attr_make_non_resident
   ntfs_resident_attr_resize
   ntfs_attr_expand_locked
   ntfs_expand_for_write
   ntfs_file_write_iter
   vfs_write

With debug output the values were attr_size 13 (unnamed $DATA) and
data_size 550 (named stream) for the same mft record.

It does not need a damaged volume. A file with an alternate data stream
whose record has no room left is enough. The fuzzer found it through a
single changed byte in a file name of an image from mkntfs, which made
a write grow a resident attribute in such a record.

Opening the stream by name instead would convert it with only the mft
record lock held. Its folio would be locked under that lock, while the
read path takes the folio lock first and the mft record lock inside
ntfs_read_iomap_begin_resident(). Streams are not converted from this
loop, which only tries to make room, so skip them.

Fixes: 495e90fa3348 ("ntfs: update attrib operations")
Cc: stable@vger.kernel.org # v7.1+
Signed-off-by: Wahid Khan <wahidzk0091@gmail.com>
---
 fs/ntfs/attrib.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/fs/ntfs/attrib.c b/fs/ntfs/attrib.c
index ce7c1b524..800e53df5 100644
--- a/fs/ntfs/attrib.c
+++ b/fs/ntfs/attrib.c
@@ -4932,6 +4932,12 @@ static int ntfs_resident_attr_resize(struct ntfs_inode *attr_ni, const s64 newsi
 			continue;
 		if (a->type == AT_DATA && !value_len)
 			continue;
+		/*
+		 * Named streams have inodes of their own and are not converted
+		 * from here.
+		 */
+		if (a->type == AT_DATA && a->name_length)
+			continue;
 
 		if (a->type == AT_DATA)
 			tvi = ntfs_iget(sb, base_ni->mft_no);
-- 
2.53.0

Re: [PATCH v2 3/3] ntfs: do not convert a named $DATA stream when making room in a record

[Wahid Khan]

Hi Namjae,

The Sashiko review of 3/3 also suggests skipping every AT_DATA
attribute in this loop, not only the named ones. I'd keep the unnamed
case as it is, for three reasons:

- Converting the unnamed $DATA from this loop is what the loop already
  did before the patch. The patch only stops it from pairing the length
  of a named stream with the unnamed inode.

- The lock order the review points at (mrec_lock held while
  ntfs_attr_make_non_resident() locks folio 0) is the same one used by
  the direct conversion near the top of ntfs_resident_attr_resize(),
  before the loop. So it is not specific to the loop.

- When an EA or a stream grows in a full record, converting the unnamed
  $DATA is what frees the space. Skipping all of $DATA would turn those
  writes into failures.

If you'd rather drop the unnamed case from the loop too, I can send
that as a separate patch so the two changes can be judged on their own.

Thanks,
Wahid

Re: [PATCH v2 3/3] ntfs: do not convert a named $DATA stream when making room in a record

[Namjae Jeon]

On Fri, Nov 6, 2026 at 10:31 PM Wahid Khan <wahidzk0091@gmail.com> wrote:
>
> Hi Namjae,
Hi Wahid,
>
> The Sashiko review of 3/3 also suggests skipping every AT_DATA
> attribute in this loop, not only the named ones. I'd keep the unnamed
> case as it is, for three reasons:
>
> - Converting the unnamed $DATA from this loop is what the loop already
>   did before the patch. The patch only stops it from pairing the length
>   of a named stream with the unnamed inode.
>
> - The lock order the review points at (mrec_lock held while
>   ntfs_attr_make_non_resident() locks folio 0) is the same one used by
>   the direct conversion near the top of ntfs_resident_attr_resize(),
>   before the loop. So it is not specific to the loop.
>
> - When an EA or a stream grows in a full record, converting the unnamed
>   $DATA is what frees the space. Skipping all of $DATA would turn those
>   writes into failures.
>
> If you'd rather drop the unnamed case from the loop too, I can send
> that as a separate patch so the two changes can be judged on their own.
I think making the lock ordering consistent and using ntfs_attr_iget()
for named $DATA would be the right approach. Please check it if you
have the time.
Thanks for the patches!

Re: [PATCH v2 0/3] ntfs: three fixes for crafted images, found by fuzzing

[Namjae Jeon]

On Fri, Nov 6, 2026 at 9:46 PM Wahid Khan <wahidzk0091@gmail.com> wrote:
>
> I fuzz the new fs/ntfs driver with mutated images: an image from mkntfs
> populated with ntfs-3g, a few bytes changed, then mount, walk the tree
> and write, under KASAN. These three problems are still present in
> ntfs-next (e62b384ca). Each patch has its trace and reproduction steps.
>
> 1. ntfs: do not read an index block outside $INDEX_ALLOCATION
>    A directory without a usable $INDEX_ALLOCATION hangs on its first
>    lookup. The task waits for an mft record lock that it already holds.
> 2. ntfs: open the index allocation before reading or writing an index block
>    NULL pointer dereference in ntfs_ib_write() on link().
> 3. ntfs: do not convert a named $DATA stream when making room in a record
>    WARN_ON() in ntfs_attr_make_non_resident(); no damaged volume needed.
>
> All three need an image to be mounted, so a crafted image, not a remote
> attacker.
>
> Changes in v2, after an automated review of v1 that I checked by hand:
> - 2/3: ntfs_ia_open() also succeeds for a directory without
>   $INDEX_ALLOCATION and returns an empty resident inode. Reject an index
>   inode that is not non-resident, as 1/3 already does.
> - 3/3: v1 opened the named stream and converted it. That would lock the
>   stream's folio under the mft record lock, the reverse of the read
>   path (ntfs_read_iomap_begin_resident()). v2 skips named streams in
>   that loop instead.
> - 1/3: unchanged.
>
> v1: https://lore.kernel.org/all/20261106091420.2461683-1-wahidzk0091@gmail.com/
>
>
> Wahid Khan (3):
>   ntfs: do not read an index block outside $INDEX_ALLOCATION
>   ntfs: open the index allocation before reading or writing an index
>     block
>   ntfs: do not convert a named $DATA stream when making room in a record
Applied it to #ntfs-next.
Thanks!