Re: [PATCH 1/4] ntfs: validate index block header more strictly

[CharSyam]

Hi, Hyunchul.

 err = ntfs_index_block_inconsistent(...) now stores the helper's -1.
Previously err was still
0 at this point, so the common exit path normalized corruption errors
to -EIO via if (!err)
err = -EIO;. With this patch, the -1 skips that normalization and is
exposed to callers as
-EPERM.

 Was that intended? If not, this path should set err = -EIO before
jumping to unm_err_out, or
ntfs_index_block_inconsistent() should be converted to return real
errno values. The latter
may be cleaner, since ntfs_index_header_inconsistent() already returns
-EIO but the caller
currently collapses it back to -1.

Thanks.
DaeMyung.

2026년 5월 22일 (금) 오전 9:48, Hyunchul Lee <hyc.lee@xxxxxxxxx>님이 작성:
>
> Modify ntfs_index_block_inconsisent() to perform stricter validation of
> INDEX_HEADER geometry in INDX blocks, and update
> ntfs_lookup_inode_by_name() to use that function to validate INDX
> blocks.
>
> Signed-off-by: Hyunchul Lee <hyc.lee@xxxxxxxxx>
> ---
>  fs/ntfs/dir.c   | 38 +++++---------------------
>  fs/ntfs/index.c | 85 +++++++++++++++++++++++++++++++++++++++++----------------
>  fs/ntfs/index.h |  3 ++
>  3 files changed, 72 insertions(+), 54 deletions(-)
>
> diff --git a/fs/ntfs/dir.c b/fs/ntfs/dir.c
> index 20f5c7074bdd..6745a0e6e3e7 100644
> --- a/fs/ntfs/dir.c
> +++ b/fs/ntfs/dir.c
> @@ -342,43 +342,19 @@ u64 ntfs_lookup_inode_by_name(struct ntfs_inode *dir_ni, const __le16 *uname,
>                         dir_ni->mft_no);
>                 goto unm_err_out;
>         }
> -       /* Catch multi sector transfer fixup errors. */
> -       if (unlikely(!ntfs_is_indx_record(ia->magic))) {
> -               ntfs_error(sb,
> -                       "Directory index record with vcn 0x%llx is corrupt.  Corrupt inode 0x%llx.  Run chkdsk.",
> -                       vcn, dir_ni->mft_no);
> -               goto unm_err_out;
> -       }
> -       if (le64_to_cpu(ia->index_block_vcn) != vcn) {
> -               ntfs_error(sb,
> -                       "Actual VCN (0x%llx) of index buffer is different from expected VCN (0x%llx). Directory inode 0x%llx is corrupt or driver bug.",
> -                       le64_to_cpu(ia->index_block_vcn),
> -                       vcn, dir_ni->mft_no);
> -               goto unm_err_out;
> -       }
> -       if (le32_to_cpu(ia->index.allocated_size) + 0x18 !=
> -                       dir_ni->itype.index.block_size) {
> -               ntfs_error(sb,
> -                       "Index buffer (VCN 0x%llx) of directory inode 0x%llx has a size (%u) differing from the directory specified size (%u). Directory inode is corrupt or driver bug.",
> -                       vcn, dir_ni->mft_no,
> -                       le32_to_cpu(ia->index.allocated_size) + 0x18,
> -                       dir_ni->itype.index.block_size);
> -               goto unm_err_out;
> -       }
>         index_end = (u8 *)ia + dir_ni->itype.index.block_size;
>         if (index_end > kaddr + PAGE_SIZE) {
>                 ntfs_error(sb,
> -                       "Index buffer (VCN 0x%llx) of directory inode 0x%llx crosses page boundary. Impossible! Cannot access! This is probably a bug in the driver.",
> -                       vcn, dir_ni->mft_no);
> +                          "Index buffer (VCN 0x%llx) of directory inode 0x%llx crosses page boundary. Impossible! Cannot access! This is probably a bug in the driver.",
> +                          vcn, dir_ni->mft_no);
>                 goto unm_err_out;
>         }
> -       index_end = (u8 *)&ia->index + le32_to_cpu(ia->index.index_length);
> -       if (index_end > (u8 *)ia + dir_ni->itype.index.block_size) {
> -               ntfs_error(sb,
> -                       "Size of index buffer (VCN 0x%llx) of directory inode 0x%llx exceeds maximum size.",
> -                       vcn, dir_ni->mft_no);
> +       err = ntfs_index_block_inconsistent(vol, ia,
> +                                           dir_ni->itype.index.block_size,
> +                                           vcn, dir_ni->mft_no);
> +       if (err)
>                 goto unm_err_out;
> -       }
> +       index_end = (u8 *)&ia->index + le32_to_cpu(ia->index.index_length);
>         /* The first index entry. */
>         ie = (struct index_entry *)((u8 *)&ia->index +
>                         le32_to_cpu(ia->index.entries_offset));
> diff --git a/fs/ntfs/index.c b/fs/ntfs/index.c
> index 146e011c1a41..c203a5ad47b6 100644
> --- a/fs/ntfs/index.c
> +++ b/fs/ntfs/index.c
> @@ -303,6 +303,53 @@ static int ntfs_ie_end(struct index_entry *ie)
>         return ie->flags & INDEX_ENTRY_END || !ie->length;
>  }
>
> +static int ntfs_index_header_inconsistent(struct ntfs_volume *vol,
> +                                         const struct index_header *ih,
> +                                         u32 bytes_available, u64 inum)
> +{
> +       u32 entries_offset = le32_to_cpu(ih->entries_offset);
> +       u32 index_length = le32_to_cpu(ih->index_length);
> +       u32 allocated_size = le32_to_cpu(ih->allocated_size);
> +
> +       if (bytes_available < sizeof(struct index_header)) {
> +               ntfs_error(vol->sb,
> +                          "index block in inode %llu is smaller than an index header.",
> +                          (unsigned long long)inum);
> +               return -EIO;
> +       }
> +
> +       if (entries_offset < sizeof(struct index_header) ||
> +           entries_offset > bytes_available) {
> +               ntfs_error(vol->sb,
> +                          "Invalid index entry offset in inode %llu.",
> +                          (unsigned long long)inum);
> +               return -EIO;
> +       }
> +
> +       if (index_length <= entries_offset) {
> +               ntfs_error(vol->sb,
> +                          "No space for index entries in inode %llu.",
> +                          (unsigned long long)inum);
> +               return -EIO;
> +       }
> +
> +       if (allocated_size < index_length) {
> +               ntfs_error(vol->sb,
> +                          "Index entries overflow in inode %llu.",
> +                          (unsigned long long)inum);
> +               return -EIO;
> +       }
> +
> +       if (allocated_size > bytes_available || index_length > bytes_available) {
> +               ntfs_error(vol->sb,
> +                          "Index entries in inode %llu exceed the available buffer.",
> +                          (unsigned long long)inum);
> +               return -EIO;
> +       }
> +
> +       return 0;
> +}
> +
>  /*
>   *  Find the last entry in the index block
>   */
> @@ -452,20 +499,19 @@ static struct index_entry *ntfs_ie_dup_novcn(struct index_entry *ie)
>   *
>   * size(struct index_header) <= ent_offset < ind_length <= alloc_size < bk_size
>   */
> -static int ntfs_index_block_inconsistent(struct ntfs_index_context *icx,
> -               struct index_block *ib, s64 vcn)
> +int ntfs_index_block_inconsistent(struct ntfs_volume *vol,
> +                                 const struct index_block *ib,
> +                                 u32 block_size, s64 vcn, u64 inum)
>  {
>         u32 ib_size = (unsigned int)le32_to_cpu(ib->index.allocated_size) +
>                 offsetof(struct index_block, index);
> -       struct super_block *sb = icx->idx_ni->vol->sb;
> -       unsigned long long inum = icx->idx_ni->mft_no;
> +       struct super_block *sb = vol->sb;
>
>         ntfs_debug("Entering\n");
>
>         if (!ntfs_is_indx_record(ib->magic)) {
> -
>                 ntfs_error(sb, "Corrupt index block signature: vcn %lld inode %llu\n",
> -                               vcn, (unsigned long long)icx->idx_ni->mft_no);
> +                          vcn, (unsigned long long)inum);
>                 return -1;
>         }
>
> @@ -477,27 +523,18 @@ static int ntfs_index_block_inconsistent(struct ntfs_index_context *icx,
>                 return -1;
>         }
>
> -       if (ib_size != icx->block_size) {
> +       if (ib_size != block_size) {
>                 ntfs_error(sb,
> -                       "Corrupt index block : s64 (%lld) of inode %llu has a size (%u) differing from the index specified size (%u)\n",
> -                       vcn, inum, ib_size, icx->block_size);
> +                          "Corrupt index block : s64 (%lld) of inode %llu has a size (%u) differing from the index specified size (%u)\n",
> +                          vcn, inum, ib_size, block_size);
>                 return -1;
>         }
>
> -       if (le32_to_cpu(ib->index.entries_offset) < sizeof(struct index_header)) {
> -               ntfs_error(sb, "Invalid index entry offset in inode %lld\n", inum);
> -               return -1;
> -       }
> -       if (le32_to_cpu(ib->index.index_length) <=
> -           le32_to_cpu(ib->index.entries_offset)) {
> -               ntfs_error(sb, "No space for index entries in inode %lld\n", inum);
> +       if (ntfs_index_header_inconsistent(vol, &ib->index,
> +                                          block_size -
> +                                          offsetof(struct index_block, index),
> +                                          inum))
>                 return -1;
> -       }
> -       if (le32_to_cpu(ib->index.allocated_size) <
> -           le32_to_cpu(ib->index.index_length)) {
> -               ntfs_error(sb, "Index entries overflow in inode %lld\n", inum);
> -               return -1;
> -       }
>
>         return 0;
>  }
> @@ -669,7 +706,9 @@ static int ntfs_ib_read(struct ntfs_index_context *icx, s64 vcn, struct index_bl
>         }
>
>         post_read_mst_fixup((struct ntfs_record *)((u8 *)dst), icx->block_size);
> -       if (ntfs_index_block_inconsistent(icx, dst, vcn))
> +       if (ntfs_index_block_inconsistent(icx->idx_ni->vol, dst,
> +                                         icx->block_size, vcn,
> +                                         icx->idx_ni->mft_no))
>                 return -1;
>
>         return 0;
> diff --git a/fs/ntfs/index.h b/fs/ntfs/index.h
> index e68d6fabaf9f..3451ec8a1c4e 100644
> --- a/fs/ntfs/index.h
> +++ b/fs/ntfs/index.h
> @@ -89,6 +89,9 @@ struct ntfs_index_context {
>         bool sync_write;
>  };
>
> +int ntfs_index_block_inconsistent(struct ntfs_volume *vol,
> +                                 const struct index_block *ib,
> +                                 u32 block_size, s64 vcn, u64 inum);
>  int ntfs_index_entry_inconsistent(struct ntfs_index_context *icx, struct ntfs_volume *vol,
>                 const struct index_entry *ie, __le32 collation_rule, u64 inum);
>  struct ntfs_index_context *ntfs_index_ctx_get(struct ntfs_inode *ni, __le16 *name,
>
> --
> 2.43.0
>
>