[PATCH] ntfs: fix out-of-bounds write in ntfs_index_walk_down()

[PATCH] ntfs: fix out-of-bounds write in ntfs_index_walk_down()

[DaeMyung Kang]

ntfs_index_walk_down() used to update the index traversal depth
directly before writing parent_pos[] and parent_vcn[]. A malformed
directory index with too many child-node levels can therefore advance
pindex past MAX_PARENT_VCN and write past the fixed arrays in struct
ntfs_index_context, corrupting context state used by later index
traversal.

Use ntfs_icx_parent_inc() for walk-down transitions so the existing
depth limit is enforced before the arrays are updated. Make the helper
check the limit before incrementing pindex so failed callers do not
leave the context at an out-of-range depth.

This is reachable by iterating a crafted NTFS directory after the volume
has been mounted, including read-only mounts. The reproducer uses
getdents64() on an index root that points to an excessively deep chain
of child index blocks.

A crafted directory index with a chain of child-node entries reproduced
UBSAN array-index-out-of-bounds reports in ntfs_index_walk_down() and
subsequent KASAN reports in ntfs_index_walk_up(). With this change, the
same image is rejected with "Index is over 32 level deep" and no KASAN
or UBSAN report is emitted.

Fixes: 0a8ac0c1fa0b ("ntfs: update directory operations")
Signed-off-by: DaeMyung Kang <charsyam@gmail.com>
---
 fs/ntfs/index.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/fs/ntfs/index.c b/fs/ntfs/index.c
index a547bdcfa456..35c4eaa4e394 100644
--- a/fs/ntfs/index.c
+++ b/fs/ntfs/index.c
@@ -677,11 +677,11 @@ static int ntfs_ib_read(struct ntfs_index_context *icx, s64 vcn, struct index_bl
 
 static int ntfs_icx_parent_inc(struct ntfs_index_context *icx)
 {
-	icx->pindex++;
-	if (icx->pindex >= MAX_PARENT_VCN) {
+	if (icx->pindex >= MAX_PARENT_VCN - 1) {
 		ntfs_error(icx->idx_ni->vol->sb, "Index is over %d level deep", MAX_PARENT_VCN);
 		return -EOPNOTSUPP;
 	}
+	icx->pindex++;
 	return 0;
 }
 
@@ -1970,6 +1970,7 @@ struct index_entry *ntfs_index_walk_down(struct index_entry *ie, struct ntfs_ind
 {
 	struct index_entry *entry;
 	struct index_block *ib;
+	int err;
 	s64 vcn;
 
 	entry = ie;
@@ -1979,14 +1980,20 @@ struct index_entry *ntfs_index_walk_down(struct index_entry *ie, struct ntfs_ind
 			ib = kvzalloc(ictx->block_size, GFP_NOFS);
 			if (!ib)
 				return ERR_PTR(-ENOMEM);
-			/* down from level zero */
+			/* is_in_root implies pindex == 0; move to the first child level. */
+			err = ntfs_icx_parent_inc(ictx);
+			if (err) {
+				kvfree(ib);
+				return ERR_PTR(err);
+			}
 			ictx->ir = NULL;
 			ictx->ib = ib;
-			ictx->pindex = 1;
 			ictx->is_in_root = false;
 		} else {
 			/* down from non-zero level */
-			ictx->pindex++;
+			err = ntfs_icx_parent_inc(ictx);
+			if (err)
+				return ERR_PTR(err);
 		}
 
 		ictx->parent_pos[ictx->pindex] = 0;
-- 
2.43.0

Re: [PATCH] ntfs: fix out-of-bounds write in ntfs_index_walk_down()

[Hyunchul Lee]

2026년 5월 7일 (목) 오전 12:38, DaeMyung Kang <charsyam@gmail.com>님이 작성:
>
> ntfs_index_walk_down() used to update the index traversal depth
> directly before writing parent_pos[] and parent_vcn[]. A malformed
> directory index with too many child-node levels can therefore advance
> pindex past MAX_PARENT_VCN and write past the fixed arrays in struct
> ntfs_index_context, corrupting context state used by later index
> traversal.
>
> Use ntfs_icx_parent_inc() for walk-down transitions so the existing
> depth limit is enforced before the arrays are updated. Make the helper
> check the limit before incrementing pindex so failed callers do not
> leave the context at an out-of-range depth.
>
> This is reachable by iterating a crafted NTFS directory after the volume
> has been mounted, including read-only mounts. The reproducer uses
> getdents64() on an index root that points to an excessively deep chain
> of child index blocks.
>
> A crafted directory index with a chain of child-node entries reproduced
> UBSAN array-index-out-of-bounds reports in ntfs_index_walk_down() and
> subsequent KASAN reports in ntfs_index_walk_up(). With this change, the
> same image is rejected with "Index is over 32 level deep" and no KASAN
> or UBSAN report is emitted.
>
> Fixes: 0a8ac0c1fa0b ("ntfs: update directory operations")
> Signed-off-by: DaeMyung Kang <charsyam@gmail.com>

Looks good to me.

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


> ---
>  fs/ntfs/index.c | 17 ++++++++++++-----
>  1 file changed, 12 insertions(+), 5 deletions(-)
>
> diff --git a/fs/ntfs/index.c b/fs/ntfs/index.c
> index a547bdcfa456..35c4eaa4e394 100644
> --- a/fs/ntfs/index.c
> +++ b/fs/ntfs/index.c
> @@ -677,11 +677,11 @@ static int ntfs_ib_read(struct ntfs_index_context *icx, s64 vcn, struct index_bl
>
>  static int ntfs_icx_parent_inc(struct ntfs_index_context *icx)
>  {
> -       icx->pindex++;
> -       if (icx->pindex >= MAX_PARENT_VCN) {
> +       if (icx->pindex >= MAX_PARENT_VCN - 1) {
>                 ntfs_error(icx->idx_ni->vol->sb, "Index is over %d level deep", MAX_PARENT_VCN);
>                 return -EOPNOTSUPP;
>         }
> +       icx->pindex++;
>         return 0;
>  }
>
> @@ -1970,6 +1970,7 @@ struct index_entry *ntfs_index_walk_down(struct index_entry *ie, struct ntfs_ind
>  {
>         struct index_entry *entry;
>         struct index_block *ib;
> +       int err;
>         s64 vcn;
>
>         entry = ie;
> @@ -1979,14 +1980,20 @@ struct index_entry *ntfs_index_walk_down(struct index_entry *ie, struct ntfs_ind
>                         ib = kvzalloc(ictx->block_size, GFP_NOFS);
>                         if (!ib)
>                                 return ERR_PTR(-ENOMEM);
> -                       /* down from level zero */
> +                       /* is_in_root implies pindex == 0; move to the first child level. */
> +                       err = ntfs_icx_parent_inc(ictx);
> +                       if (err) {
> +                               kvfree(ib);
> +                               return ERR_PTR(err);
> +                       }
>                         ictx->ir = NULL;
>                         ictx->ib = ib;
> -                       ictx->pindex = 1;
>                         ictx->is_in_root = false;
>                 } else {
>                         /* down from non-zero level */
> -                       ictx->pindex++;
> +                       err = ntfs_icx_parent_inc(ictx);
> +                       if (err)
> +                               return ERR_PTR(err);
>                 }
>
>                 ictx->parent_pos[ictx->pindex] = 0;
> --
> 2.43.0
>


--
Thanks,
Hyunchul

Re: [PATCH] ntfs: fix out-of-bounds write in ntfs_index_walk_down()

[Namjae Jeon]

> @@ -1979,14 +1980,20 @@ struct index_entry *ntfs_index_walk_down(struct index_entry *ie, struct ntfs_ind
>                         ib = kvzalloc(ictx->block_size, GFP_NOFS);
>                         if (!ib)
>                                 return ERR_PTR(-ENOMEM);
> -                       /* down from level zero */
> +                       /* is_in_root implies pindex == 0; move to the first child level. */
> +                       err = ntfs_icx_parent_inc(ictx);
> +                       if (err) {
> +                               kvfree(ib);
> +                               return ERR_PTR(err);
> +                       }

At this point is_in_root is true, so ->pindex is guaranteed to be 0.
Calling ntfs_icx_parent_inc() which includes an overflow check and
error handling is unnecessary here, and the error handling code
becomes dead code. Directly setting the value will be simpler and
clearer like this.

/*
 * Descending from root index (level 0) to the first child level.
 * is_in_root == true implies pindex == 0, so advance to level 1.
 */
ictx->pindex = 1;

[PATCH v2] ntfs: fix out-of-bounds write in ntfs_index_walk_down()

[DaeMyung Kang]

ntfs_index_walk_down() used to update the index traversal depth
directly before writing parent_pos[] and parent_vcn[]. A malformed
directory index with too many child-node levels can therefore advance
pindex past MAX_PARENT_VCN and write past the fixed arrays in struct
ntfs_index_context, corrupting context state used by later index
traversal.

Use ntfs_icx_parent_inc() for walk-down transitions so the existing
depth limit is enforced before the arrays are updated. Make the helper
check the limit before incrementing pindex so failed callers do not
leave the context at an out-of-range depth.

This is reachable by iterating a crafted NTFS directory after the volume
has been mounted, including read-only mounts. The reproducer uses
getdents64() on an index root that points to an excessively deep chain
of child index blocks.

A crafted directory index with a chain of child-node entries reproduced
UBSAN array-index-out-of-bounds reports in ntfs_index_walk_down() and
subsequent KASAN reports in ntfs_index_walk_up(). With this change, the
same image is rejected with "Index is over 32 level deep" and no KASAN
or UBSAN report is emitted.

Fixes: 0a8ac0c1fa0b ("ntfs: update directory operations")
Suggested-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: DaeMyung Kang <charsyam@gmail.com>
---
Changes since v1:
  - In the is_in_root walk-down branch, drop the ntfs_icx_parent_inc()
    call and the kvfree(ib) error handling; restore the direct
    ictx->pindex = 1 assignment, per review by Namjae Jeon. The helper
    call there was unnecessary because is_in_root == true implies
    pindex == 0, and the resulting error path was dead code. Add a
    comment documenting the invariant. The non-root walk-down branch
    keeps using ntfs_icx_parent_inc() since pindex can approach
    MAX_PARENT_VCN there.

 fs/ntfs/index.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/fs/ntfs/index.c b/fs/ntfs/index.c
index a547bdcfa456..146e011c1a41 100644
--- a/fs/ntfs/index.c
+++ b/fs/ntfs/index.c
@@ -677,11 +677,11 @@ static int ntfs_ib_read(struct ntfs_index_context *icx, s64 vcn, struct index_bl
 
 static int ntfs_icx_parent_inc(struct ntfs_index_context *icx)
 {
-	icx->pindex++;
-	if (icx->pindex >= MAX_PARENT_VCN) {
+	if (icx->pindex >= MAX_PARENT_VCN - 1) {
 		ntfs_error(icx->idx_ni->vol->sb, "Index is over %d level deep", MAX_PARENT_VCN);
 		return -EOPNOTSUPP;
 	}
+	icx->pindex++;
 	return 0;
 }
 
@@ -1970,6 +1970,7 @@ struct index_entry *ntfs_index_walk_down(struct index_entry *ie, struct ntfs_ind
 {
 	struct index_entry *entry;
 	struct index_block *ib;
+	int err;
 	s64 vcn;
 
 	entry = ie;
@@ -1979,14 +1980,20 @@ struct index_entry *ntfs_index_walk_down(struct index_entry *ie, struct ntfs_ind
 			ib = kvzalloc(ictx->block_size, GFP_NOFS);
 			if (!ib)
 				return ERR_PTR(-ENOMEM);
-			/* down from level zero */
+			/*
+			 * Descending from root index (level 0) to the first
+			 * child level. is_in_root == true implies pindex == 0,
+			 * so advance to level 1.
+			 */
+			ictx->pindex = 1;
 			ictx->ir = NULL;
 			ictx->ib = ib;
-			ictx->pindex = 1;
 			ictx->is_in_root = false;
 		} else {
 			/* down from non-zero level */
-			ictx->pindex++;
+			err = ntfs_icx_parent_inc(ictx);
+			if (err)
+				return ERR_PTR(err);
 		}
 
 		ictx->parent_pos[ictx->pindex] = 0;
-- 
2.43.0