Re: [PATCH v2 1/2] ntfs: fix NULL dereference in ntfs_index_walk_down()

[Hyunchul Lee]

2026년 4월 26일 (일) 오후 1:02, DaeMyung Kang <charsyam@xxxxxxxxx>님이 작성:
>
> 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@xxxxxxxxx>

Looks good to me.

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

> ---
>  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..a547bdcfa456 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(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
>


-- 
Thanks,
Hyunchul