Re: [PATCH] ntfs: avoid calling post_write_mst_fixup() for invalid index_block

[Hyunchul Lee]

2026년 7월 4일 (토) 오후 6:39, Valeriy Yashnikov <yashnikov.valeriy@xxxxxxxxx>님이 작성:
>
> ntfs_icx_ib_sync_write() calls post_write_mst_fixup() when ntfs_ib_write()
> returns an error, intending to restore the buffer after a failed write.
>
> However, ntfs_ib_write() returns an error immediately if
> pre_write_mst_fixup() validation fails. The caller,
> ntfs_icx_ib_sync_write(), interprets any error as a write failure
> requiring rollback. It does not differentiate between I/O errors and
> validation failures, and calls post_write_mst_fixup() anyway.
>
> Since post_write_mst_fixup() assumes that the index_block contents is
> correct, it doesn't perform the boundary checks, which results in
> out-of-bounds memory access.
>
> An attacker can craft a malicious NTFS image with:
>   - large index_block.usa_ofs offset, pointing outside the ntfs_record
>   - index_block.usa_count = 0, causing integer underflow
>   - or index_block.usa_count larger than actual number of sectors in the
>     ntfs_record, causing out-of-bounds access
>
> KASAN reports describing the memory corruption:
>   ==================================================================
>   BUG: KASAN: slab-out-of-bounds in post_write_mst_fixup+0x19c/0x1d0
>   Read of size 2 at addr ffff8881586c9018 by task p/9428
>   Call Trace:
>    <TASK>
>    dump_stack_lvl+0x100/0x190
>    print_report+0x139/0x4ad
>    ? post_write_mst_fixup+0x19c/0x1d0
>    ? __virt_addr_valid+0x262/0x500
>    ? post_write_mst_fixup+0x19c/0x1d0
>    kasan_report+0xe4/0x1d0
>    ? post_write_mst_fixup+0x19c/0x1d0
>    post_write_mst_fixup+0x19c/0x1d0
>    ntfs_icx_ib_sync_write+0x179/0x220
>    ntfs_inode_sync_filename+0x83d/0x1080
>    __ntfs_write_inode+0x1049/0x1480
>    ntfs_file_fsync+0x131/0x9b0
>   ==================================================================
>   BUG: KASAN: slab-out-of-bounds in post_write_mst_fixup+0x1aa/0x1d0
>   Write of size 2 at addr ffff8881586c91fe by task p/9428
>   Call Trace:
>    <TASK>
>    dump_stack_lvl+0x100/0x190
>    print_report+0x139/0x4ad
>    ? post_write_mst_fixup+0x1aa/0x1d0
>    ? __virt_addr_valid+0x262/0x500
>    ? post_write_mst_fixup+0x1aa/0x1d0
>    kasan_report+0xe4/0x1d0
>    ? post_write_mst_fixup+0x1aa/0x1d0
>    post_write_mst_fixup+0x1aa/0x1d0
>    ntfs_icx_ib_sync_write+0x179/0x220
>    ntfs_inode_sync_filename+0x83d/0x1080
>    __ntfs_write_inode+0x1049/0x1480
>    ntfs_file_fsync+0x131/0x9b0
>   ==================================================================
>
> Let's move the post_write_mst_fixup() call to ntfs_ib_write().
> The ntfs_ib_write() function calls pre_write_mst_fixup() at the beginning.
> If the index_block contents is invalid, pre_write_mst_fixup() fails and
> ntfs_ib_write() returns early without calling post_write_mst_fixup() on
> bad index_block.
>
> Fixes: 0a8ac0c1fa0b ("ntfs: update directory operations")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Valeriy Yashnikov <yashnikov.valeriy@xxxxxxxxx>

Looks good to me.

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

> ---
>  fs/ntfs/index.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/fs/ntfs/index.c b/fs/ntfs/index.c
> index c5f2cf75b750..faa7ee920a3a 100644
> --- a/fs/ntfs/index.c
> +++ b/fs/ntfs/index.c
> @@ -110,6 +110,10 @@ static int ntfs_ib_write(struct ntfs_index_context *icx, struct index_block *ib)
>         ret = ntfs_inode_attr_pwrite(VFS_I(icx->ia_ni),
>                         ntfs_ib_vcn_to_pos(icx, vcn), icx->block_size,
>                         (u8 *)ib, icx->sync_write);
> +
> +       /* Perform data restoration before returning */
> +       post_write_mst_fixup((struct ntfs_record *)ib);
> +
>         if (ret != icx->block_size) {
>                 ntfs_debug("Failed to write index block %lld, inode %llu",
>                                 vcn, (unsigned long long)icx->idx_ni->mft_no);
> @@ -147,7 +151,6 @@ int ntfs_icx_ib_sync_write(struct ntfs_index_context *icx)
>                 icx->ib = NULL;
>                 icx->ib_dirty = false;
>         } else {
> -               post_write_mst_fixup((struct ntfs_record *)icx->ib);
>                 icx->sync_write = false;
>         }
>
> --
> 2.54.0
>


-- 
Thanks,
Hyunchul