ntfs: validate index entries on reading

Validate index entries immediately after reading an index root or index
block from disk. This eliminates repeated checks in lookup and readdir,
and reduce the risk of missing checks in those paths.

Cc: stable@vger.kernel.org # v7.1
Tested-by: woot000 <woot000@woot000.com>
Signed-off-by: Hyunchul Lee <hyc.lee@gmail.com>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>

diff --git a/fs/ntfs/dir.c b/fs/ntfs/dir.c
index 6745a0e6e3e76d51726f5baaeffda819a0750bbe..4b6bd5f30c65ea422dc119ef4b2303b4859d3d8d 100644 (file)
--- a/fs/ntfs/dir.c
+++ b/fs/ntfs/dir.c
@@ -135,10 +135,6 @@ u64 ntfs_lookup_inode_by_name(struct ntfs_inode *dir_ni, const __le16 *uname,
                /* Key length should not be zero if it is not last entry. */
                if (!ie->key_length)
                        goto dir_err_out;
-               /* Check the consistency of an index entry */
-               if (ntfs_index_entry_inconsistent(NULL, vol, ie, COLLATION_FILE_NAME,
-                               dir_ni->mft_no))
-                       goto dir_err_out;
                /*
                 * We perform a case sensitive comparison and if that matches
                 * we are done and return the mft reference of the inode (i.e.
@@ -351,7 +347,8 @@ fast_descend_into_child_node:
        }
        err = ntfs_index_block_inconsistent(vol, ia,
                                            dir_ni->itype.index.block_size,
-                                           vcn, dir_ni->mft_no);
+                                           vcn, COLLATION_FILE_NAME,
+                                           dir_ni->mft_no);
        if (err)
                goto unm_err_out;
        index_end = (u8 *)&ia->index + le32_to_cpu(ia->index.index_length);
@@ -364,15 +361,6 @@ fast_descend_into_child_node:
         * reach the last entry.
         */
        for (;; ie = (struct index_entry *)((u8 *)ie + le16_to_cpu(ie->length))) {
-               /* Bounds checks. */
-               if ((u8 *)ie < (u8 *)ia ||
-                   (u8 *)ie + sizeof(struct index_entry_header) > index_end ||
-                   (u8 *)ie + sizeof(struct index_entry_header) + le16_to_cpu(ie->key_length) >
-                               index_end || (u8 *)ie + le16_to_cpu(ie->length) > index_end) {
-                       ntfs_error(sb, "Index entry out of bounds in directory inode 0x%llx.",
-                                       dir_ni->mft_no);
-                       goto unm_err_out;
-               }
                /*
                 * The last entry cannot contain a name. It can however contain
                 * a pointer to a child node in the B+tree so we just break out.
@@ -382,10 +370,6 @@ fast_descend_into_child_node:
                /* Key length should not be zero if it is not last entry. */
                if (!ie->key_length)
                        goto unm_err_out;
-               /* Check the consistency of an index entry */
-               if (ntfs_index_entry_inconsistent(NULL, vol, ie, COLLATION_FILE_NAME,
-                               dir_ni->mft_no))
-                       goto unm_err_out;
                /*
                 * We perform a case sensitive comparison and if that matches
                 * we are done and return the mft reference of the inode (i.e.
@@ -868,6 +852,7 @@ static int ntfs_readdir(struct file *file, struct dir_context *actor)
                ictx->vcn_size_bits = vol->cluster_size_bits;
        else
                ictx->vcn_size_bits = NTFS_BLOCK_SIZE_BITS;
+       ictx->cr = ir->collation_rule;
 
        /* The first index entry. */
        next = (struct index_entry *)((u8 *)&ir->index +
@@ -905,13 +890,6 @@ static int ntfs_readdir(struct file *file, struct dir_context *actor)
                if (!next)
                        break;
 nextdir:
-               /* Check the consistency of an index entry */
-               if (ntfs_index_entry_inconsistent(ictx, vol, next, COLLATION_FILE_NAME,
-                                       ndir->mft_no)) {
-                       err = -EIO;
-                       goto out;
-               }
-
                if (ie_pos < actor->pos) {
                        ie_pos += le16_to_cpu(next->length);
                        continue;

diff --git a/fs/ntfs/index.c b/fs/ntfs/index.c
index 97c0e7d6a580bf96396c3c91f2907b9bb288122e..00e17637f77184f7ec177e7872cc4f3c7c93c62c 100644 (file)
--- a/fs/ntfs/index.c
+++ b/fs/ntfs/index.c
@@ -28,41 +28,10 @@
  * length must have been checked beforehand to not overflow from the
  * index record.
  */
-int ntfs_index_entry_inconsistent(struct ntfs_index_context *icx,
-               struct ntfs_volume *vol, const struct index_entry *ie,
-               __le32 collation_rule, u64 inum)
+static int ntfs_index_entry_inconsistent(const struct ntfs_volume *vol,
+                                        const struct index_entry *ie,
+                                        __le32 collation_rule, u64 inum)
 {
-       if (icx) {
-               struct index_header *ih;
-               u8 *ie_start, *ie_end;
-
-               if (icx->is_in_root)
-                       ih = &icx->ir->index;
-               else
-                       ih = &icx->ib->index;
-
-               if ((le32_to_cpu(ih->index_length) > le32_to_cpu(ih->allocated_size)) ||
-                               (le32_to_cpu(ih->index_length) > icx->block_size)) {
-                       ntfs_error(vol->sb, "%s Index entry(0x%p)'s length is too big.",
-                                       icx->is_in_root ? "Index root" : "Index block",
-                                       (u8 *)icx->entry);
-                       return -EINVAL;
-               }
-
-               ie_start = (u8 *)ih + le32_to_cpu(ih->entries_offset);
-               ie_end = (u8 *)ih + le32_to_cpu(ih->index_length);
-
-               if (ie_start > (u8 *)ie ||
-                   ie_end <= (u8 *)ie + le16_to_cpu(ie->length) ||
-                   le16_to_cpu(ie->length) > le32_to_cpu(ih->allocated_size) ||
-                   le16_to_cpu(ie->length) > icx->block_size) {
-                       ntfs_error(vol->sb, "Index entry(0x%p) is out of range from %s",
-                                       (u8 *)icx->entry,
-                                       icx->is_in_root ? "index root" : "index block");
-                       return -EIO;
-               }
-       }
-
        if (ie->key_length &&
            ((le16_to_cpu(ie->key_length) + offsetof(struct index_entry, key)) >
             le16_to_cpu(ie->length))) {
@@ -352,6 +321,44 @@ static int ntfs_index_header_inconsistent(struct ntfs_volume *vol,
        return 0;
 }
 
+int ntfs_index_entries_inconsistent(const struct ntfs_volume *vol,
+                                   const struct index_header *ih,
+                                   __le32 collation_rule, u64 inum)
+{
+       struct index_entry *ie;
+       u8 *index_end = (u8 *)ih + le32_to_cpu(ih->index_length);
+
+       for (ie = ntfs_ie_get_first((struct index_header *)ih);
+             ; ie = ntfs_ie_get_next(ie)) {
+               if ((u8 *)ie + sizeof(struct index_entry_header) > index_end ||
+                   (u8 *)ie + le16_to_cpu(ie->length) > index_end) {
+                       ntfs_error(vol->sb,
+                                  "Index entry out of bounds in inode %llu.",
+                                  (unsigned long long)inum);
+                       return -EIO;
+               }
+
+               if (le16_to_cpu(ie->length) < sizeof(struct index_entry_header)) {
+                       ntfs_error(vol->sb,
+                                  "Index etnry too small in inode %llu.",
+                                  inum);
+                       return -EIO;
+               }
+
+               if (ntfs_ie_end(ie))
+                       break;
+
+               if (!ie->key_length)
+                       return -EIO;
+
+               if (ntfs_index_entry_inconsistent(vol, ie,
+                                                 collation_rule, inum))
+                       return -EIO;
+       }
+
+       return 0;
+}
+
 /*
  *  Find the last entry in the index block
  */
@@ -503,7 +510,8 @@ static struct index_entry *ntfs_ie_dup_novcn(struct index_entry *ie)
  */
 int ntfs_index_block_inconsistent(struct ntfs_volume *vol,
                                  const struct index_block *ib,
-                                 u32 block_size, s64 vcn, u64 inum)
+                                 u32 block_size, s64 vcn, __le32 cr,
+                                 u64 inum)
 {
        u32 ib_size = (unsigned int)le32_to_cpu(ib->index.allocated_size) +
                offsetof(struct index_block, index);
@@ -537,7 +545,8 @@ int ntfs_index_block_inconsistent(struct ntfs_volume *vol,
                                           offsetof(struct index_block, index),
                                           inum))
                return -EIO;
-
+       if (ntfs_index_entries_inconsistent(vol, &ib->index, cr, inum))
+               return -EIO;
        return 0;
 }
 
@@ -727,10 +736,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->idx_ni->vol, dst,
-                                         icx->block_size, vcn,
+                                         icx->block_size, vcn, icx->cr,
                                          icx->idx_ni->mft_no))
                return -EIO;
-
        return 0;
 }
 

diff --git a/fs/ntfs/index.h b/fs/ntfs/index.h
index cad78568d8b350f99158d8b520064e0e6715d903..9a03f53bba47919b5995ce7daf781563b6505e33 100644 (file)
--- a/fs/ntfs/index.h
+++ b/fs/ntfs/index.h
@@ -94,9 +94,11 @@ int ntfs_index_root_inconsistent(struct ntfs_volume *vol,
                                 const struct index_root *ir, u64 inum);
 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);
+                                 u32 block_size, s64 vcn,
+                                 __le32 cr, u64 inum);
+int ntfs_index_entries_inconsistent(const struct ntfs_volume *vol,
+                                   const struct index_header *ih,
+                                   __le32 collation_rule, u64 inum);
 struct ntfs_index_context *ntfs_index_ctx_get(struct ntfs_inode *ni, __le16 *name,
                u32 name_len);
 void ntfs_index_ctx_put(struct ntfs_index_context *ictx);

diff --git a/fs/ntfs/inode.c b/fs/ntfs/inode.c
index 63ee7acff4fc92963092c03b0ab7b35d66ac8d61..9717fb5b47097e925e53f7f87df49742badc4c4e 100644 (file)
--- a/fs/ntfs/inode.c
+++ b/fs/ntfs/inode.c
@@ -939,7 +939,9 @@ view_index_meta:
                }
                ir = (struct index_root *)((u8 *)a +
                                le16_to_cpu(a->data.resident.value_offset));
-               if (ntfs_index_root_inconsistent(ni->vol, a, ir, ni->mft_no)) {
+               if (ntfs_index_root_inconsistent(ni->vol, a, ir, ni->mft_no) ||
+                   ntfs_index_entries_inconsistent(ni->vol, &ir->index,
+                                                   ir->collation_rule, ni->mft_no)) {
                        ntfs_error(vi->i_sb, "Directory index is corrupt.");
                        goto unm_err_out;
                }
@@ -1529,7 +1531,9 @@ static int ntfs_read_locked_index_inode(struct inode *base_vi, struct inode *vi)
        }
 
        ir = (struct index_root *)((u8 *)a + le16_to_cpu(a->data.resident.value_offset));
-       if (ntfs_index_root_inconsistent(vol, a, ir, ni->mft_no)) {
+       if (ntfs_index_root_inconsistent(vol, a, ir, ni->mft_no) ||
+           ntfs_index_entries_inconsistent(vol, &ir->index,
+                                           ir->collation_rule, ni->mft_no)) {
                ntfs_error(vi->i_sb, "Index is corrupt.");
                goto unm_err_out;
        }