public inbox for linux-fsdevel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] ntfs: fix index walk NULL deref and WSL symlink leak
@ 2026-04-25 18:42 DaeMyung Kang
  2026-04-25 18:42 ` [PATCH 1/2] ntfs: fix NULL dereference in ntfs_index_walk_down() DaeMyung Kang
  2026-04-25 18:42 ` [PATCH 2/2] ntfs: fix WSL symlink target leak on reparse failure DaeMyung Kang
  0 siblings, 2 replies; 5+ messages in thread
From: DaeMyung Kang @ 2026-04-25 18:42 UTC (permalink / raw)
  To: Namjae Jeon, Hyunchul Lee; +Cc: linux-fsdevel, linux-kernel, DaeMyung Kang

Two independent fixes for the new fs/ntfs/ that landed in v7.1-rc1.

  1/2 fixes a NULL dereference in ntfs_index_walk_down(). When
      kvzalloc() for ictx->ib fails, or ntfs_ib_read() fails mid
      traversal, the function previously returned a state that
      ntfs_index_next() and ntfs_readdir() could not distinguish
      from end-of-directory, and ntfs_ib_read() itself could write
      through a NULL ictx->ib. Errors are now propagated as
      ERR_PTR() through ntfs_index_next() up to ntfs_readdir().
      Reproduced with failslab fault injection on getdents64; the
      reproducer is described in the commit log.

  2/2 fixes a target-string leak in ntfs_reparse_set_wsl_symlink()
      when ntfs_set_ntfs_reparse_data() fails. Also switches the
      kvfree() on the local failure path to kfree() to match the
      kmalloc() done by ntfs_ucstonls().

The two patches are independent and may be applied in any order.

DaeMyung Kang (2):
  ntfs: fix NULL dereference in ntfs_index_walk_down()
  ntfs: fix WSL symlink target leak on reparse failure

 fs/ntfs/dir.c     | 13 ++++++++++---
 fs/ntfs/index.c   | 17 +++++++++++++----
 fs/ntfs/reparse.c |  5 +++--
 3 files changed, 26 insertions(+), 9 deletions(-)

--
2.43.0


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

* [PATCH 1/2] ntfs: fix NULL dereference in ntfs_index_walk_down()
  2026-04-25 18:42 [PATCH 0/2] ntfs: fix index walk NULL deref and WSL symlink leak DaeMyung Kang
@ 2026-04-25 18:42 ` DaeMyung Kang
  2026-04-26  1:01   ` Namjae Jeon
  2026-04-26  3:47   ` DaeMyung Kang
  2026-04-25 18:42 ` [PATCH 2/2] ntfs: fix WSL symlink target leak on reparse failure DaeMyung Kang
  1 sibling, 2 replies; 5+ messages in thread
From: DaeMyung Kang @ 2026-04-25 18:42 UTC (permalink / raw)
  To: Namjae Jeon, Hyunchul Lee; +Cc: linux-fsdevel, linux-kernel, DaeMyung Kang

ntfs_index_walk_down() allocates ictx->ib when descending from the root
into an index allocation block. If that allocation fails, the old code
still passes the NULL buffer to ntfs_ib_read(), which can write through
it via ntfs_inode_attr_pread().

Allocate the index block into a temporary pointer and return -ENOMEM
before changing the index context on allocation failure. Also propagate
ERR_PTR() through ntfs_index_next() and ntfs_readdir() so walk-down
allocation or index block read failures are not mistaken for normal
index iteration inside the filesystem.

ntfs_readdir() keeps the existing userspace-visible behavior of
suppressing readdir errors after marking end_in_iterate; this change only
prevents the walk-down failure path from dereferencing NULL internally.

The failure was reproduced with failslab fail-nth injection on getdents64;
the original module hits a NULL pointer dereference in memcpy_orig through
ntfs_ib_read(), while the patched module reaches the same
ntfs_index_walk_down() allocation failure without crashing.

Fixes: 0a8ac0c1fa0b ("ntfs: update directory operations")
Signed-off-by: DaeMyung Kang <charsyam@gmail.com>
---
 fs/ntfs/dir.c   | 13 ++++++++++---
 fs/ntfs/index.c | 17 +++++++++++++----
 2 files changed, 23 insertions(+), 7 deletions(-)

diff --git a/fs/ntfs/dir.c b/fs/ntfs/dir.c
index bfa904d2ce66..20f5c7074bdd 100644
--- a/fs/ntfs/dir.c
+++ b/fs/ntfs/dir.c
@@ -911,8 +911,8 @@ static int ntfs_readdir(struct file *file, struct dir_context *actor)
 
 	if (next->flags & INDEX_ENTRY_NODE) {
 		next = ntfs_index_walk_down(next, ictx);
-		if (!next) {
-			err = -EIO;
+		if (IS_ERR(next)) {
+			err = PTR_ERR(next);
 			goto out;
 		}
 	}
@@ -920,7 +920,14 @@ static int ntfs_readdir(struct file *file, struct dir_context *actor)
 	if (next && !(next->flags & INDEX_ENTRY_END))
 		goto nextdir;
 
-	while ((next = ntfs_index_next(next, ictx)) != NULL) {
+	while (1) {
+		next = ntfs_index_next(next, ictx);
+		if (IS_ERR(next)) {
+			err = PTR_ERR(next);
+			goto out;
+		}
+		if (!next)
+			break;
 nextdir:
 		/* Check the consistency of an index entry */
 		if (ntfs_index_entry_inconsistent(ictx, vol, next, COLLATION_FILE_NAME,
diff --git a/fs/ntfs/index.c b/fs/ntfs/index.c
index 2080f3969137..f50082708bd1 100644
--- a/fs/ntfs/index.c
+++ b/fs/ntfs/index.c
@@ -1969,15 +1969,19 @@ int ntfs_index_remove(struct ntfs_inode *dir_ni, const void *key, const u32 keyl
 struct index_entry *ntfs_index_walk_down(struct index_entry *ie, struct ntfs_index_context *ictx)
 {
 	struct index_entry *entry;
+	struct index_block *ib;
 	s64 vcn;
 
 	entry = ie;
 	do {
 		vcn = ntfs_ie_get_vcn(entry);
 		if (ictx->is_in_root) {
+			ib = kvzalloc(ictx->block_size, GFP_NOFS);
+			if (!ib)
+				return ERR_PTR(-ENOMEM);
 			/* down from level zero */
 			ictx->ir = NULL;
-			ictx->ib = kvzalloc(ictx->block_size, GFP_NOFS);
+			ictx->ib = ib;
 			ictx->pindex = 1;
 			ictx->is_in_root = false;
 		} else {
@@ -1991,8 +1995,8 @@ struct index_entry *ntfs_index_walk_down(struct index_entry *ie, struct ntfs_ind
 			ictx->entry = ntfs_ie_get_first(&ictx->ib->index);
 			entry = ictx->entry;
 		} else
-			entry = NULL;
-	} while (entry && (entry->flags & INDEX_ENTRY_NODE));
+			entry = ERR_PTR(-EIO);
+	} while (!IS_ERR_OR_NULL(entry) && (entry->flags & INDEX_ENTRY_NODE));
 
 	return entry;
 }
@@ -2097,10 +2101,15 @@ struct index_entry *ntfs_index_next(struct index_entry *ie, struct ntfs_index_co
 
 		/* walk down if it has a subnode */
 		if (flags & INDEX_ENTRY_NODE) {
-			if (!ictx->ia_ni)
+			if (!ictx->ia_ni) {
 				ictx->ia_ni = ntfs_ia_open(ictx, ictx->idx_ni);
+				if (!ictx->ia_ni)
+					return ERR_PTR(-EIO);
+			}
 
 			next = ntfs_index_walk_down(next, ictx);
+			if (IS_ERR(next))
+				return next;
 		} else {
 
 			/* walk up it has no subnode, nor data */
-- 
2.43.0


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

* [PATCH 2/2] ntfs: fix WSL symlink target leak on reparse failure
  2026-04-25 18:42 [PATCH 0/2] ntfs: fix index walk NULL deref and WSL symlink leak DaeMyung Kang
  2026-04-25 18:42 ` [PATCH 1/2] ntfs: fix NULL dereference in ntfs_index_walk_down() DaeMyung Kang
@ 2026-04-25 18:42 ` DaeMyung Kang
  1 sibling, 0 replies; 5+ messages in thread
From: DaeMyung Kang @ 2026-04-25 18:42 UTC (permalink / raw)
  To: Namjae Jeon, Hyunchul Lee; +Cc: linux-fsdevel, linux-kernel, DaeMyung Kang

ntfs_reparse_set_wsl_symlink() converts the symlink target into an
allocated NLS string and transfers ownership to ni->target only after
ntfs_set_ntfs_reparse_data() succeeds. If setting the reparse data fails,
the converted target is left unreferenced and leaks.

Free the converted target on the reparse update failure path. Use kfree()
for the other local failure path as well, matching the ntfs_ucstonls()
allocation contract.

Fixes: fc053f05ca28 ("ntfs: add reparse and ea operations")
Signed-off-by: DaeMyung Kang <charsyam@gmail.com>
---
 fs/ntfs/reparse.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/fs/ntfs/reparse.c b/fs/ntfs/reparse.c
index 8f60ec6f66c1..74713716813f 100644
--- a/fs/ntfs/reparse.c
+++ b/fs/ntfs/reparse.c
@@ -505,7 +505,6 @@ int ntfs_reparse_set_wsl_symlink(struct ntfs_inode *ni,
 	struct reparse_point *reparse;
 	struct wsl_link_reparse_data *data;
 
-	utarget = (char *)NULL;
 	len = ntfs_ucstonls(ni->vol, target, target_len, &utarget, 0);
 	if (len <= 0)
 		return -EINVAL;
@@ -514,7 +513,7 @@ int ntfs_reparse_set_wsl_symlink(struct ntfs_inode *ni,
 	reparse = kvzalloc(reparse_len, GFP_NOFS);
 	if (!reparse) {
 		err = -ENOMEM;
-		kvfree(utarget);
+		kfree(utarget);
 	} else {
 		data = (struct wsl_link_reparse_data *)reparse->reparse_data;
 		reparse->reparse_tag = IO_REPARSE_TAG_LX_SYMLINK;
@@ -528,6 +527,8 @@ int ntfs_reparse_set_wsl_symlink(struct ntfs_inode *ni,
 		kvfree(reparse);
 		if (!err)
 			ni->target = utarget;
+		else
+			kfree(utarget);
 	}
 	return err;
 }
-- 
2.43.0


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

* Re: [PATCH 1/2] ntfs: fix NULL dereference in ntfs_index_walk_down()
  2026-04-25 18:42 ` [PATCH 1/2] ntfs: fix NULL dereference in ntfs_index_walk_down() DaeMyung Kang
@ 2026-04-26  1:01   ` Namjae Jeon
  2026-04-26  3:47   ` DaeMyung Kang
  1 sibling, 0 replies; 5+ messages in thread
From: Namjae Jeon @ 2026-04-26  1:01 UTC (permalink / raw)
  To: DaeMyung Kang; +Cc: Hyunchul Lee, linux-fsdevel, linux-kernel

> @@ -1991,8 +1995,8 @@ struct index_entry *ntfs_index_walk_down(struct index_entry *ie, struct ntfs_ind
>                         ictx->entry = ntfs_ie_get_first(&ictx->ib->index);
>                         entry = ictx->entry;
>                 } else
> -                       entry = NULL;
> -       } while (entry && (entry->flags & INDEX_ENTRY_NODE));
> +                       entry = ERR_PTR(-EIO);
> +       } while (!IS_ERR_OR_NULL(entry) && (entry->flags & INDEX_ENTRY_NODE));
Is there any reason why you use IS_ERR_OR_NULL(), not IS_ERR() ?

Thanks.

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

* Re: [PATCH 1/2] ntfs: fix NULL dereference in ntfs_index_walk_down()
  2026-04-25 18:42 ` [PATCH 1/2] ntfs: fix NULL dereference in ntfs_index_walk_down() DaeMyung Kang
  2026-04-26  1:01   ` Namjae Jeon
@ 2026-04-26  3:47   ` DaeMyung Kang
  1 sibling, 0 replies; 5+ messages in thread
From: DaeMyung Kang @ 2026-04-26  3:47 UTC (permalink / raw)
  To: Namjae Jeon; +Cc: Hyunchul Lee, linux-fsdevel, linux-kernel, DaeMyung Kang

On Sat, Apr 25, 2026, Namjae Jeon wrote:
> Is there any reason why you use IS_ERR_OR_NULL(), not IS_ERR() ?

No good reason -- that was over-defensive on my part. Inside the loop,
entry is only set to one of:

  - ie, the caller's argument (both call sites pass a pointer
    arithmetic result that cannot be NULL),
  - ntfs_ie_get_first(&ictx->ib->index), which is also pointer
    arithmetic on ictx->ib that we just checked non-NULL above,
  - ERR_PTR(-EIO) on ntfs_ib_read() failure.

So entry is never NULL inside the loop body. I'll switch to plain
IS_ERR() in v2.

Thanks,
DaeMyung

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

end of thread, other threads:[~2026-04-26  3:48 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-25 18:42 [PATCH 0/2] ntfs: fix index walk NULL deref and WSL symlink leak DaeMyung Kang
2026-04-25 18:42 ` [PATCH 1/2] ntfs: fix NULL dereference in ntfs_index_walk_down() DaeMyung Kang
2026-04-26  1:01   ` Namjae Jeon
2026-04-26  3:47   ` DaeMyung Kang
2026-04-25 18:42 ` [PATCH 2/2] ntfs: fix WSL symlink target leak on reparse failure DaeMyung Kang

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