Re: [PATCH v6 2/4] ntfs: grow index root value before reparent header update

[Hyunchul Lee]

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

Looks good to me.

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

> ---
>  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