Harden superblock cache deserialization (#2809)

1 files changed, 146 insertions, 214 deletions

diff --git a/src/H5Fsuper_cache.c b/src/H5Fsuper_cache.c
index 467e287..7dbaf22 100644
--- a/src/H5Fsuper_cache.c
+++ b/src/H5Fsuper_cache.c
@@ -13,10 +13,8 @@
 /*-------------------------------------------------------------------------
  *
  * Created:		H5Fsuper_cache.c
- *			Aug 15 2009
- *			Quincey Koziol
  *
- * Purpose:		Implement file superblock & driver info metadata cache methods.
+ * Purpose:		Implement file superblock & driver info metadata cache methods
  *
  *-------------------------------------------------------------------------
  */
@@ -76,10 +74,10 @@ static herr_t H5F__cache_drvrinfo_serialize(const H5F_t *f, void *image, size_t
 static herr_t H5F__cache_drvrinfo_free_icr(void *thing);
 
 /* Local encode/decode routines */
-static herr_t H5F__superblock_prefix_decode(H5F_super_t *sblock, const uint8_t **image_ref,
+static herr_t H5F__superblock_prefix_decode(H5F_super_t *sblock, const uint8_t **image_ref, size_t len,
                                             const H5F_superblock_cache_ud_t *udata, hbool_t extend_eoa);
 static herr_t H5F__drvrinfo_prefix_decode(H5O_drvinfo_t *drvinfo, char *drv_name, const uint8_t **image_ref,
-                                          H5F_drvrinfo_cache_ud_t *udata, hbool_t extend_eoa);
+                                          size_t len, H5F_drvrinfo_cache_ud_t *udata, hbool_t extend_eoa);
 
 /*********************/
 /* Package Variables */
@@ -135,25 +133,21 @@ H5FL_EXTERN(H5F_super_t);
 /*-------------------------------------------------------------------------
  * Function:    H5F__superblock_prefix_decode
  *
- * Purpose:	Decode a superblock prefix
- *
- * Return:      Non-negative on success/Negative on failure
- *
- * Programmer:  Quincey Koziol
- *              December 15, 2016
+ * Purpose:     Decode a superblock prefix
  *
+ * Return:      SUCCEED/FAIL
  *-------------------------------------------------------------------------
  */
 static herr_t
-H5F__superblock_prefix_decode(H5F_super_t *sblock, const uint8_t **image_ref,
+H5F__superblock_prefix_decode(H5F_super_t *sblock, const uint8_t **image_ref, size_t len,
                               const H5F_superblock_cache_ud_t *udata, hbool_t extend_eoa)
 {
     const uint8_t *image     = (const uint8_t *)*image_ref; /* Pointer into raw data buffer */
-    htri_t         ret_value = SUCCEED;                     /* Return value */
+    const uint8_t *end       = image + len - 1;             /* Pointer to end of buffer */
+    htri_t         ret_value = SUCCEED;
 
     FUNC_ENTER_PACKAGE
 
-    /* Check arguments */
     HDassert(sblock);
     HDassert(image_ref);
     HDassert(image);
@@ -161,27 +155,37 @@ H5F__superblock_prefix_decode(H5F_super_t *sblock, const uint8_t **image_ref,
     HDassert(udata->f);
 
     /* Skip over signature (already checked when locating the superblock) */
+    if (H5_IS_BUFFER_OVERFLOW(image, H5F_SIGNATURE_LEN, end))
+        HGOTO_ERROR(H5E_FILE, H5E_OVERFLOW, FAIL, "ran off end of input buffer while decoding");
     image += H5F_SIGNATURE_LEN;
 
     /* Superblock version */
+    if (H5_IS_BUFFER_OVERFLOW(image, 1, end))
+        HGOTO_ERROR(H5E_FILE, H5E_OVERFLOW, FAIL, "ran off end of input buffer while decoding");
     sblock->super_vers = *image++;
     if (sblock->super_vers > HDF5_SUPERBLOCK_VERSION_LATEST)
         HGOTO_ERROR(H5E_FILE, H5E_BADVALUE, FAIL, "bad superblock version number")
 
-    /* Sanity check */
-    HDassert(((size_t)(image - (const uint8_t *)*image_ref)) == H5F_SUPERBLOCK_FIXED_SIZE);
+    /* Size check */
+    if (((size_t)(image - (const uint8_t *)*image_ref)) != H5F_SUPERBLOCK_FIXED_SIZE)
+        HGOTO_ERROR(H5E_FILE, H5E_BADVALUE, FAIL, "bad superblock (fixed) size")
 
     /* Determine the size of addresses & size of offsets, for computing the
      * variable-sized portion of the superblock.
      */
     if (sblock->super_vers < HDF5_SUPERBLOCK_VERSION_2) {
+        if (H5_IS_BUFFER_OVERFLOW(image, 6, end))
+            HGOTO_ERROR(H5E_FILE, H5E_OVERFLOW, FAIL, "ran off end of input buffer while decoding");
         sblock->sizeof_addr = image[4];
         sblock->sizeof_size = image[5];
-    } /* end if */
+    }
     else {
+        if (H5_IS_BUFFER_OVERFLOW(image, 2, end))
+            HGOTO_ERROR(H5E_FILE, H5E_OVERFLOW, FAIL, "ran off end of input buffer while decoding");
         sblock->sizeof_addr = image[0];
         sblock->sizeof_size = image[1];
-    } /* end else */
+    }
+
     if (sblock->sizeof_addr != 2 && sblock->sizeof_addr != 4 && sblock->sizeof_addr != 8 &&
         sblock->sizeof_addr != 16 && sblock->sizeof_addr != 32)
         HGOTO_ERROR(H5E_FILE, H5E_BADVALUE, FAIL, "bad byte number in an address")
@@ -196,12 +200,13 @@ H5F__superblock_prefix_decode(H5F_super_t *sblock, const uint8_t **image_ref,
         /* Determine the size of the variable-length part of the superblock */
         variable_size =
             (size_t)H5F_SUPERBLOCK_VARLEN_SIZE(sblock->super_vers, sblock->sizeof_addr, sblock->sizeof_size);
-        HDassert(variable_size > 0);
+        if (variable_size == 0)
+            HGOTO_ERROR(H5E_FILE, H5E_BADVALUE, FAIL, "variable size can't be zero")
 
         /* Make certain we can read the variable-sized portion of the superblock */
         if (H5F__set_eoa(udata->f, H5FD_MEM_SUPER, (haddr_t)(H5F_SUPERBLOCK_FIXED_SIZE + variable_size)) < 0)
             HGOTO_ERROR(H5E_FILE, H5E_CANTINIT, FAIL, "set end of space allocation request failed")
-    } /* end if */
+    }
 
     /* Update the image buffer pointer */
     *image_ref = image;
@@ -211,28 +216,24 @@ done:
 } /* end H5F__superblock_prefix_decode() */
 
 /*-------------------------------------------------------------------------
- * Function:	H5F__drvrinfo_prefix_decode
- *
- * Purpose:	Decode a driver info prefix
+ * Function:    H5F__drvrinfo_prefix_decode
  *
- * Return:      Non-negative on success/Negative on failure
- *
- * Programmer:  Quincey Koziol
- *              December 15, 2016
+ * Purpose:     Decode a driver info prefix
  *
+ * Return:      SUCCEED/FAIL
  *-------------------------------------------------------------------------
  */
 static herr_t
-H5F__drvrinfo_prefix_decode(H5O_drvinfo_t *drvrinfo, char *drv_name, const uint8_t **image_ref,
+H5F__drvrinfo_prefix_decode(H5O_drvinfo_t *drvrinfo, char *drv_name, const uint8_t **image_ref, size_t len,
                             H5F_drvrinfo_cache_ud_t *udata, hbool_t extend_eoa)
 {
     const uint8_t *image = (const uint8_t *)*image_ref; /* Pointer into raw data buffer */
+    const uint8_t *end   = image + len - 1;             /* Pointer to end of buffer */
     unsigned       drv_vers;                            /* Version of driver info block */
-    herr_t         ret_value = SUCCEED;                 /* Return value */
+    herr_t         ret_value = SUCCEED;
 
     FUNC_ENTER_PACKAGE
 
-    /* Sanity check */
     HDassert(drvrinfo);
     HDassert(image_ref);
     HDassert(image);
@@ -240,21 +241,30 @@ H5F__drvrinfo_prefix_decode(H5O_drvinfo_t *drvrinfo, char *drv_name, const uint8
     HDassert(udata->f);
 
     /* Version number */
+    if (H5_IS_BUFFER_OVERFLOW(image, 1, end))
+        HGOTO_ERROR(H5E_FILE, H5E_OVERFLOW, FAIL, "ran off end of input buffer while decoding");
     drv_vers = *image++;
     if (drv_vers != HDF5_DRIVERINFO_VERSION_0)
         HGOTO_ERROR(H5E_FILE, H5E_BADVALUE, FAIL, "bad driver information block version number")
 
-    image += 3; /* reserved bytes */
+    /* Reserved bytes */
+    if (H5_IS_BUFFER_OVERFLOW(image, 3, end))
+        HGOTO_ERROR(H5E_FILE, H5E_OVERFLOW, FAIL, "ran off end of input buffer while decoding");
+    image += 3;
 
     /* Driver info size */
+    if (H5_IS_BUFFER_OVERFLOW(image, 4, end))
+        HGOTO_ERROR(H5E_FILE, H5E_OVERFLOW, FAIL, "ran off end of input buffer while decoding");
     UINT32DECODE(image, drvrinfo->len);
 
     /* Driver name and/or version */
     if (drv_name) {
+        if (H5_IS_BUFFER_OVERFLOW(image, 8, end))
+            HGOTO_ERROR(H5E_FILE, H5E_OVERFLOW, FAIL, "ran off end of input buffer while decoding");
         H5MM_memcpy(drv_name, (const char *)image, (size_t)8);
         drv_name[8] = '\0';
         image += 8; /* advance past name/version */
-    }               /* end if */
+    }
 
     /* Extend the EOA if required so that we can read the complete driver info block */
     if (extend_eoa) {
@@ -273,7 +283,7 @@ H5F__drvrinfo_prefix_decode(H5O_drvinfo_t *drvrinfo, char *drv_name, const uint8
         if (H5F_addr_gt(min_eoa, eoa))
             if (H5FD_set_eoa(udata->f->shared->lf, H5FD_MEM_SUPER, min_eoa) < 0)
                 HGOTO_ERROR(H5E_FILE, H5E_CANTINIT, FAIL, "set end of space allocation request failed")
-    } /* end if */
+    }
 
     /* Update the image buffer pointer */
     *image_ref = image;
@@ -285,13 +295,9 @@ done:
 /*-------------------------------------------------------------------------
  * Function:    H5F__cache_superblock_get_initial_load_size
  *
- * Purpose:     Compute the size of the data structure on disk.
- *
- * Return:      Non-negative on success/Negative on failure
- *
- * Programmer:  Quincey Koziol
- *              July 17, 2013
+ * Purpose:     Compute the size of the data structure on disk
  *
+ * Return:      SUCCEED/FAIL
  *-------------------------------------------------------------------------
  */
 static herr_t
@@ -299,7 +305,6 @@ H5F__cache_superblock_get_initial_load_size(void H5_ATTR_UNUSED *_udata, size_t
 {
     FUNC_ENTER_PACKAGE_NOERR
 
-    /* Check arguments */
     HDassert(image_len);
 
     /* Set the initial image length size */
@@ -312,27 +317,22 @@ H5F__cache_superblock_get_initial_load_size(void H5_ATTR_UNUSED *_udata, size_t
 /*-------------------------------------------------------------------------
  * Function:    H5F__cache_superblock_get_final_load_size
  *
- * Purpose:     Compute the final size of the data structure on disk.
- *
- * Return:      Non-negative on success/Negative on failure
- *
- * Programmer:  Quincey Koziol
- *              November 17, 2016
+ * Purpose:     Compute the final size of the data structure on disk
  *
+ * Return:      SUCCEED/FAIL
  *-------------------------------------------------------------------------
  */
 static herr_t
-H5F__cache_superblock_get_final_load_size(const void *_image, size_t H5_ATTR_NDEBUG_UNUSED image_len,
-                                          void *_udata, size_t *actual_len)
+H5F__cache_superblock_get_final_load_size(const void *_image, size_t image_len, void *_udata,
+                                          size_t *actual_len)
 {
     const uint8_t             *image = _image;                              /* Pointer into raw data buffer */
     H5F_superblock_cache_ud_t *udata = (H5F_superblock_cache_ud_t *)_udata; /* User data */
     H5F_super_t                sblock;                                      /* Temporary file superblock */
-    htri_t                     ret_value = SUCCEED;                         /* Return value */
+    htri_t                     ret_value = SUCCEED;
 
     FUNC_ENTER_PACKAGE
 
-    /* Check arguments */
     HDassert(image);
     HDassert(udata);
     HDassert(actual_len);
@@ -340,7 +340,7 @@ H5F__cache_superblock_get_final_load_size(const void *_image, size_t H5_ATTR_NDE
     HDassert(image_len >= H5F_SUPERBLOCK_FIXED_SIZE + 6);
 
     /* Deserialize the file superblock's prefix */
-    if (H5F__superblock_prefix_decode(&sblock, &image, udata, TRUE) < 0)
+    if (H5F__superblock_prefix_decode(&sblock, &image, image_len, udata, TRUE) < 0)
         HGOTO_ERROR(H5E_FILE, H5E_CANTDECODE, FAIL, "can't decode file superblock prefix")
 
     /* Save the version to be used in verify_chksum callback */
@@ -357,14 +357,11 @@ done:
 /*-------------------------------------------------------------------------
  * Function:    H5F__cache_superblock_verify_chksum
  *
- * Purpose:	Verify the computed checksum of the data structure is the
- *		same as the stored chksum.
- *
- * Return:      Success:        TRUE/FALSE
- *              Failure:        Negative
- *
- * Programmer:  Vailin Choi; Aug 2015
+ * Purpose:     Verify the computed checksum of the data structure is the
+ *              same as the stored chksum.
  *
+ * Return:      Success:    TRUE/FALSE
+ *              Failure:    Negative
  *-------------------------------------------------------------------------
  */
 static htri_t
@@ -372,13 +369,12 @@ H5F__cache_superblock_verify_chksum(const void *_image, size_t len, void *_udata
 {
     const uint8_t             *image = _image;                              /* Pointer into raw data buffer */
     H5F_superblock_cache_ud_t *udata = (H5F_superblock_cache_ud_t *)_udata; /* User data */
-    uint32_t                   stored_chksum;    /* Stored metadata checksum value */
-    uint32_t                   computed_chksum;  /* Computed metadata checksum value */
-    htri_t                     ret_value = TRUE; /* Return value */
+    uint32_t                   stored_chksum;   /* Stored metadata checksum value */
+    uint32_t                   computed_chksum; /* Computed metadata checksum value */
+    htri_t                     ret_value = TRUE;
 
     FUNC_ENTER_PACKAGE_NOERR
 
-    /* Check arguments */
     HDassert(image);
     HDassert(udata);
 
@@ -390,36 +386,31 @@ H5F__cache_superblock_verify_chksum(const void *_image, size_t len, void *_udata
 
         if (stored_chksum != computed_chksum)
             ret_value = FALSE;
-    } /* end if */
+    }
 
     FUNC_LEAVE_NOAPI(ret_value)
 } /* end H5F__cache_superblock_verify_chksum() */
 
 /*-------------------------------------------------------------------------
- * Function:	H5F__cache_superblock_deserialize
- *
- * Purpose:	Loads an object from the disk.
+ * Function:    H5F__cache_superblock_deserialize
  *
- * Return:	Success:	Pointer to new object
- *		Failure:	NULL
- *
- * Programmer:	Quincey Koziol
- *		July 18 2013
+ * Purpose:     Load an object from the disk
  *
+ * Return:      Success:    Pointer to new object
+ *              Failure:    NULL
  *-------------------------------------------------------------------------
  */
 static void *
-H5F__cache_superblock_deserialize(const void *_image, size_t H5_ATTR_NDEBUG_UNUSED len, void *_udata,
-                                  hbool_t H5_ATTR_UNUSED *dirty)
+H5F__cache_superblock_deserialize(const void *_image, size_t len, void *_udata, hbool_t H5_ATTR_UNUSED *dirty)
 {
     H5F_super_t               *sblock    = NULL;                                /* File's superblock */
     H5F_superblock_cache_ud_t *udata     = (H5F_superblock_cache_ud_t *)_udata; /* User data */
-    const uint8_t             *image     = _image; /* Pointer into raw data buffer */
-    H5F_super_t               *ret_value = NULL;   /* Return value */
+    const uint8_t             *image     = _image;          /* Pointer into raw data buffer */
+    const uint8_t             *end       = image + len - 1; /* Pointer to end of buffer */
+    H5F_super_t               *ret_value = NULL;
 
     FUNC_ENTER_PACKAGE
 
-    /* Check arguments */
     HDassert(image);
     HDassert(udata);
     HDassert(udata->f);
@@ -430,11 +421,9 @@ H5F__cache_superblock_deserialize(const void *_image, size_t H5_ATTR_NDEBUG_UNUS
         HGOTO_ERROR(H5E_RESOURCE, H5E_NOSPACE, NULL, "memory allocation failed")
 
     /* Deserialize the file superblock's prefix */
-    if (H5F__superblock_prefix_decode(sblock, &image, udata, FALSE) < 0)
+    if (H5F__superblock_prefix_decode(sblock, &image, len, udata, FALSE) < 0)
         HGOTO_ERROR(H5E_FILE, H5E_CANTDECODE, NULL, "can't decode file superblock prefix")
 
-    const uint8_t *image_end = image + len - 1;
-
     /* Check for older version of superblock format */
     if (sblock->super_vers < HDF5_SUPERBLOCK_VERSION_2) {
         uint32_t status_flags;  /* File status flags	   */
@@ -442,122 +431,113 @@ H5F__cache_superblock_deserialize(const void *_image, size_t H5_ATTR_NDEBUG_UNUS
         unsigned snode_btree_k; /* B-tree symbol table internal node 'K' value */
         unsigned chunk_btree_k; /* B-tree chunk internal node 'K' value */
 
-        /* Check whether the image pointer is out of bounds */
-        if (H5_IS_BUFFER_OVERFLOW(image, 1, image_end))
-            HGOTO_ERROR(H5E_FILE, H5E_OVERFLOW, NULL, "image pointer is out of bounds")
-
         /* Freespace version (hard-wired) */
+        if (H5_IS_BUFFER_OVERFLOW(image, 1, end))
+            HGOTO_ERROR(H5E_FILE, H5E_OVERFLOW, NULL, "image pointer is out of bounds")
         if (HDF5_FREESPACE_VERSION != *image++)
             HGOTO_ERROR(H5E_FILE, H5E_BADVALUE, NULL, "bad free space version number")
 
-        /* Check whether the image pointer is out of bounds */
-        if (H5_IS_BUFFER_OVERFLOW(image, 1, image_end))
-            HGOTO_ERROR(H5E_FILE, H5E_OVERFLOW, NULL, "image pointer is out of bounds")
-
         /* Root group version number (hard-wired) */
+        if (H5_IS_BUFFER_OVERFLOW(image, 1, end))
+            HGOTO_ERROR(H5E_FILE, H5E_OVERFLOW, NULL, "image pointer is out of bounds")
         if (HDF5_OBJECTDIR_VERSION != *image++)
             HGOTO_ERROR(H5E_FILE, H5E_BADVALUE, NULL, "bad object directory version number")
 
         /* Skip over reserved byte */
-        image++;
-
-        /* Check whether the image pointer is out of bounds */
-        if (H5_IS_BUFFER_OVERFLOW(image, 1, image_end))
+        if (H5_IS_BUFFER_OVERFLOW(image, 1, end))
             HGOTO_ERROR(H5E_FILE, H5E_OVERFLOW, NULL, "image pointer is out of bounds")
+        image++;
 
         /* Shared header version number (hard-wired) */
+        if (H5_IS_BUFFER_OVERFLOW(image, 1, end))
+            HGOTO_ERROR(H5E_FILE, H5E_OVERFLOW, NULL, "image pointer is out of bounds")
         if (HDF5_SHAREDHEADER_VERSION != *image++)
             HGOTO_ERROR(H5E_FILE, H5E_BADVALUE, NULL, "bad shared-header format version number")
 
         /* Skip over size of file addresses (already decoded) */
+        if (H5_IS_BUFFER_OVERFLOW(image, 1, end))
+            HGOTO_ERROR(H5E_FILE, H5E_OVERFLOW, NULL, "image pointer is out of bounds")
         image++;
         udata->f->shared->sizeof_addr = sblock->sizeof_addr; /* Keep a local copy also */
 
         /* Skip over size of file sizes (already decoded) */
+        if (H5_IS_BUFFER_OVERFLOW(image, 1, end))
+            HGOTO_ERROR(H5E_FILE, H5E_OVERFLOW, NULL, "image pointer is out of bounds")
         image++;
         udata->f->shared->sizeof_size = sblock->sizeof_size; /* Keep a local copy also */
 
         /* Skip over reserved byte */
-        image++;
-
-        /* Check whether the image pointer is out of bounds */
-        if (H5_IS_BUFFER_OVERFLOW(image, sizeof(uint16_t), image_end))
+        if (H5_IS_BUFFER_OVERFLOW(image, 1, end))
             HGOTO_ERROR(H5E_FILE, H5E_OVERFLOW, NULL, "image pointer is out of bounds")
+        image++;
 
         /* Various B-tree sizes */
+        if (H5_IS_BUFFER_OVERFLOW(image, 2, end))
+            HGOTO_ERROR(H5E_FILE, H5E_OVERFLOW, NULL, "image pointer is out of bounds")
         UINT16DECODE(image, sym_leaf_k);
         if (sym_leaf_k == 0)
             HGOTO_ERROR(H5E_FILE, H5E_BADRANGE, NULL, "bad symbol table leaf node 1/2 rank")
         udata->sym_leaf_k = sym_leaf_k; /* Keep a local copy also */
 
-        /* Check whether the image pointer is out of bounds */
-        if (H5_IS_BUFFER_OVERFLOW(image, sizeof(uint16_t), image_end))
-            HGOTO_ERROR(H5E_FILE, H5E_OVERFLOW, NULL, "image pointer is out of bounds")
-
         /* Need 'get' call to set other array values */
+        if (H5_IS_BUFFER_OVERFLOW(image, 2, end))
+            HGOTO_ERROR(H5E_FILE, H5E_OVERFLOW, NULL, "image pointer is out of bounds")
         UINT16DECODE(image, snode_btree_k);
         if (snode_btree_k == 0)
             HGOTO_ERROR(H5E_FILE, H5E_BADRANGE, NULL, "bad 1/2 rank for btree internal nodes")
         udata->btree_k[H5B_SNODE_ID] = snode_btree_k;
 
-        /*
-         * Delay setting the value in the property list until we've checked
+        /* Delay setting the value in the property list until we've checked
          * for the indexed storage B-tree internal 'K' value later.
          */
 
-        /* Check whether the image pointer is out of bounds */
-        if (H5_IS_BUFFER_OVERFLOW(image, sizeof(uint32_t), image_end))
-            HGOTO_ERROR(H5E_FILE, H5E_OVERFLOW, NULL, "image pointer is out of bounds")
-
         /* File status flags (not really used yet) */
+        if (H5_IS_BUFFER_OVERFLOW(image, 4, end))
+            HGOTO_ERROR(H5E_FILE, H5E_OVERFLOW, NULL, "image pointer is out of bounds")
         UINT32DECODE(image, status_flags);
-        HDassert(status_flags <= 255);
+        if (status_flags > 255)
+            HGOTO_ERROR(H5E_FILE, H5E_BADVALUE, NULL, "bad superblock status flags")
         sblock->status_flags = (uint8_t)status_flags;
         if (sblock->status_flags & ~H5F_SUPER_ALL_FLAGS)
             HGOTO_ERROR(H5E_FILE, H5E_BADVALUE, NULL, "bad flag value for superblock")
 
-        /*
-         * If the superblock version # is greater than 0, read in the indexed
+        /* If the superblock version # is greater than 0, read in the indexed
          * storage B-tree internal 'K' value
          */
         if (sblock->super_vers > HDF5_SUPERBLOCK_VERSION_DEF) {
-            /* Check whether the image pointer is out of bounds */
-            if (H5_IS_BUFFER_OVERFLOW(image, sizeof(uint16_t), image_end))
+            if (H5_IS_BUFFER_OVERFLOW(image, 2, end))
                 HGOTO_ERROR(H5E_FILE, H5E_OVERFLOW, NULL, "image pointer is out of bounds")
-
             UINT16DECODE(image, chunk_btree_k);
 
             /* Reserved bytes are present only in version 1 */
             if (sblock->super_vers == HDF5_SUPERBLOCK_VERSION_1) {
-                image += 2; /* reserved */
-
-                /* Check whether the image pointer is out of bounds */
-                if (H5_IS_BUFFER_OVERFLOW(image, 1, image_end))
+                /* Reserved */
+                if (H5_IS_BUFFER_OVERFLOW(image, 2, end))
                     HGOTO_ERROR(H5E_FILE, H5E_OVERFLOW, NULL, "image pointer is out of bounds")
+                image += 2;
             }
-        } /* end if */
+        }
         else
             chunk_btree_k = HDF5_BTREE_CHUNK_IK_DEF;
         udata->btree_k[H5B_CHUNK_ID] = chunk_btree_k;
 
-        /* Check whether the image pointer will be out of bounds */
-        if (H5_IS_BUFFER_OVERFLOW(image, H5F_SIZEOF_ADDR(udata->f) * 4, image_end))
-            HGOTO_ERROR(H5E_FILE, H5E_OVERFLOW, NULL, "image pointer is out of bounds")
-
         /* Remainder of "variable-sized" portion of superblock */
+        if (H5_IS_BUFFER_OVERFLOW(image, H5F_sizeof_addr(udata->f) * 4, end))
+            HGOTO_ERROR(H5E_FILE, H5E_OVERFLOW, NULL, "image pointer is out of bounds")
         H5F_addr_decode(udata->f, (const uint8_t **)&image, &sblock->base_addr /*out*/);
         H5F_addr_decode(udata->f, (const uint8_t **)&image, &sblock->ext_addr /*out*/);
         H5F_addr_decode(udata->f, (const uint8_t **)&image, &udata->stored_eof /*out*/);
         H5F_addr_decode(udata->f, (const uint8_t **)&image, &sblock->driver_addr /*out*/);
 
         /* Allocate space for the root group symbol table entry */
-        HDassert(!sblock->root_ent);
+        if (sblock->root_ent)
+            HGOTO_ERROR(H5E_FILE, H5E_BADVALUE, NULL, "root entry should not exist yet")
         if (NULL == (sblock->root_ent = (H5G_entry_t *)H5MM_calloc(sizeof(H5G_entry_t))))
             HGOTO_ERROR(H5E_FILE, H5E_CANTALLOC, NULL,
                         "can't allocate space for root group symbol table entry")
 
-        /* decode the root group symbol table entry */
-        if (H5G_ent_decode(udata->f, (const uint8_t **)&image, sblock->root_ent, image_end) < 0)
+        /* Decode the root group symbol table entry */
+        if (H5G_ent_decode(udata->f, (const uint8_t **)&image, sblock->root_ent, end) < 0)
             HGOTO_ERROR(H5E_FILE, H5E_CANTDECODE, NULL, "can't decode root group symbol table entry")
 
         /* Set the root group address to the correct value */
@@ -572,11 +552,10 @@ H5F__cache_superblock_deserialize(const void *_image, size_t H5_ATTR_NDEBUG_UNUS
             /* Eliminate the driver info */
             sblock->driver_addr     = HADDR_UNDEF;
             udata->drvrinfo_removed = TRUE;
-        } /* end if */
+        }
 
         /* NOTE: Driver info block is decoded separately, later */
-
-    } /* end if */
+    }
     else {
         uint32_t read_chksum; /* Checksum read from file  */
 
@@ -588,7 +567,7 @@ H5F__cache_superblock_deserialize(const void *_image, size_t H5_ATTR_NDEBUG_UNUS
         udata->f->shared->sizeof_size = sblock->sizeof_size; /* Keep a local copy also */
 
         /* Check whether the image pointer is out of bounds */
-        if (H5_IS_BUFFER_OVERFLOW(image, 1, image_end))
+        if (H5_IS_BUFFER_OVERFLOW(image, 1, end))
             HGOTO_ERROR(H5E_FILE, H5E_OVERFLOW, NULL, "image pointer is out of bounds")
 
         /* File status flags (not really used yet) */
@@ -597,7 +576,7 @@ H5F__cache_superblock_deserialize(const void *_image, size_t H5_ATTR_NDEBUG_UNUS
             HGOTO_ERROR(H5E_FILE, H5E_BADVALUE, NULL, "bad flag value for superblock")
 
         /* Check whether the image pointer will be out of bounds */
-        if (H5_IS_BUFFER_OVERFLOW(image, H5F_SIZEOF_ADDR(udata->f) * 4, image_end))
+        if (H5_IS_BUFFER_OVERFLOW(image, H5F_SIZEOF_ADDR(udata->f) * 4, end))
             HGOTO_ERROR(H5E_FILE, H5E_OVERFLOW, NULL, "image pointer is out of bounds")
 
         /* Base, superblock extension, end of file & root group object header addresses */
@@ -609,7 +588,7 @@ H5F__cache_superblock_deserialize(const void *_image, size_t H5_ATTR_NDEBUG_UNUS
         /* checksum verification already done in verify_chksum cb */
 
         /* Check whether the image pointer will be out of bounds */
-        if (H5_IS_BUFFER_OVERFLOW(image, sizeof(uint32_t), image_end))
+        if (H5_IS_BUFFER_OVERFLOW(image, sizeof(uint32_t), end))
             HGOTO_ERROR(H5E_FILE, H5E_OVERFLOW, NULL, "image pointer is out of bounds")
 
         /* Decode checksum */
@@ -621,12 +600,12 @@ H5F__cache_superblock_deserialize(const void *_image, size_t H5_ATTR_NDEBUG_UNUS
          * any attempt to load the Driver Information Block.
          */
         sblock->driver_addr = HADDR_UNDEF;
-    } /* end else */
+    }
 
-    /* Sanity check */
-    HDassert((size_t)(image - (const uint8_t *)_image) <= len);
+    /* Size check */
+    if ((size_t)(image - (const uint8_t *)_image) > len)
+        HDONE_ERROR(H5E_FILE, H5E_BADVALUE, NULL, "bad decoded superblock size")
 
-    /* Set return value */
     ret_value = sblock;
 
 done:
@@ -641,13 +620,9 @@ done:
 /*-------------------------------------------------------------------------
  * Function:    H5F__cache_superblock_image_len
  *
- * Purpose:     Compute the size of the data structure on disk.
- *
- * Return:      Non-negative on success/Negative on failure
- *
- * Programmer:  Quincey Koziol
- *              July 19, 2013
+ * Purpose:     Compute the size of the data structure on disk
  *
+ * Return:      SUCCEED/FAIL
  *-------------------------------------------------------------------------
  */
 static herr_t
@@ -657,7 +632,6 @@ H5F__cache_superblock_image_len(const void *_thing, size_t *image_len)
 
     FUNC_ENTER_PACKAGE_NOERR
 
-    /* Check arguments */
     HDassert(sblock);
     HDassert(sblock->cache_info.magic == H5C__H5C_CACHE_ENTRY_T_MAGIC);
     HDassert(sblock->cache_info.type == H5AC_SUPERBLOCK);
@@ -672,13 +646,9 @@ H5F__cache_superblock_image_len(const void *_thing, size_t *image_len)
 /*-------------------------------------------------------------------------
  * Function:	H5F__cache_superblock_serialize
  *
- * Purpose:	Flushes a dirty object to disk.
- *
- * Return:	Non-negative on success/Negative on failure
- *
- * Programmer:	Quincey Koziol
- *		July 19 2013
+ * Purpose:     Flush a dirty object to disk
  *
+ * Return:      SUCCEED/FAIL
  *-------------------------------------------------------------------------
  */
 static herr_t
@@ -687,11 +657,10 @@ H5F__cache_superblock_serialize(const H5F_t *f, void *_image, size_t H5_ATTR_UNU
     H5F_super_t *sblock = (H5F_super_t *)_thing; /* Pointer to the object */
     uint8_t     *image  = _image;                /* Pointer into raw data buffer */
     haddr_t      rel_eof;                        /* Relative EOF for file */
-    herr_t       ret_value = SUCCEED;            /* Return value */
+    herr_t       ret_value = SUCCEED;
 
     FUNC_ENTER_PACKAGE
 
-    /* Sanity check */
     HDassert(f);
     HDassert(image);
     HDassert(sblock);
@@ -796,7 +765,7 @@ H5F__cache_superblock_serialize(const H5F_t *f, void *_image, size_t H5_ATTR_UNU
 
         /* Sanity check */
         HDassert((size_t)(image - (uint8_t *)_image) == (size_t)H5F_SUPERBLOCK_SIZE(sblock));
-    } /* end else */
+    }
 
     /* Sanity check */
     HDassert((size_t)(image - (uint8_t *)_image) == len);
@@ -808,29 +777,24 @@ done:
 /*-------------------------------------------------------------------------
  * Function:	H5F__cache_superblock_free_icr
  *
- * Purpose:	Destroy/release an "in core representation" of a data
+ * Purpose:     Destroy/release an "in core representation" of a data
  *              structure
  *
- * Note:	The metadata cache sets the object's cache_info.magic to
- *		H5C__H5C_CACHE_ENTRY_T_BAD_MAGIC before calling a free_icr
- *		callback (checked in assert).
- *
- * Return:	Non-negative on success/Negative on failure
- *
- * Programmer:	Quincey Koziol
- *              July 20, 2013
+ * Note:        The metadata cache sets the object's cache_info.magic to
+ *              H5C__H5C_CACHE_ENTRY_T_BAD_MAGIC before calling a free_icr
+ *              callback (checked in assert).
  *
+ * Return:      SUCCEED/FAIL
  *-------------------------------------------------------------------------
  */
 static herr_t
 H5F__cache_superblock_free_icr(void *_thing)
 {
     H5F_super_t *sblock    = (H5F_super_t *)_thing; /* Pointer to the object */
-    herr_t       ret_value = SUCCEED;               /* Return value */
+    herr_t       ret_value = SUCCEED;
 
     FUNC_ENTER_PACKAGE
 
-    /* Sanity check */
     HDassert(sblock);
     HDassert(sblock->cache_info.magic == H5C__H5C_CACHE_ENTRY_T_BAD_MAGIC);
     HDassert(sblock->cache_info.type == H5AC_SUPERBLOCK);
@@ -848,11 +812,7 @@ done:
  *
  * Purpose:     Compute the initial size of the data structure on disk.
  *
- * Return:      Non-negative on success/Negative on failure
- *
- * Programmer:  Quincey Koziol
- *              July 20, 2013
- *
+ * Return:      SUCCEED/FAIL
  *-------------------------------------------------------------------------
  */
 static herr_t
@@ -860,7 +820,6 @@ H5F__cache_drvrinfo_get_initial_load_size(void H5_ATTR_UNUSED *_udata, size_t *i
 {
     FUNC_ENTER_PACKAGE_NOERR
 
-    /* Check arguments */
     HDassert(image_len);
 
     /* Set the initial image length size */
@@ -874,25 +833,20 @@ H5F__cache_drvrinfo_get_initial_load_size(void H5_ATTR_UNUSED *_udata, size_t *i
  *
  * Purpose:     Compute the final size of the data structure on disk.
  *
- * Return:      Non-negative on success/Negative on failure
- *
- * Programmer:  Quincey Koziol
- *              November 17, 2016
- *
+ * Return:      SUCCEED/FAIL
  *-------------------------------------------------------------------------
  */
 static herr_t
-H5F__cache_drvrinfo_get_final_load_size(const void *_image, size_t H5_ATTR_NDEBUG_UNUSED image_len,
-                                        void *_udata, size_t *actual_len)
+H5F__cache_drvrinfo_get_final_load_size(const void *_image, size_t image_len, void *_udata,
+                                        size_t *actual_len)
 {
     const uint8_t           *image = _image;                            /* Pointer into raw data buffer */
     H5F_drvrinfo_cache_ud_t *udata = (H5F_drvrinfo_cache_ud_t *)_udata; /* User data */
     H5O_drvinfo_t            drvrinfo;                                  /* Driver info */
-    herr_t                   ret_value = SUCCEED;                       /* Return value */
+    herr_t                   ret_value = SUCCEED;
 
     FUNC_ENTER_PACKAGE
 
-    /* Check arguments */
     HDassert(image);
     HDassert(udata);
     HDassert(actual_len);
@@ -900,7 +854,7 @@ H5F__cache_drvrinfo_get_final_load_size(const void *_image, size_t H5_ATTR_NDEBU
     HDassert(image_len == H5F_DRVINFOBLOCK_HDR_SIZE);
 
     /* Deserialize the file driver info's prefix */
-    if (H5F__drvrinfo_prefix_decode(&drvrinfo, NULL, &image, udata, TRUE) < 0)
+    if (H5F__drvrinfo_prefix_decode(&drvrinfo, NULL, &image, image_len, udata, TRUE) < 0)
         HGOTO_ERROR(H5E_FILE, H5E_CANTDECODE, FAIL, "can't decode file driver info prefix")
 
     /* Set the final size for the cache image */
@@ -913,29 +867,23 @@ done:
 /*-------------------------------------------------------------------------
  * Function:	H5F__cache_drvrinfo_deserialize
  *
- * Purpose:	Loads an object from the disk.
- *
- * Return:	Success:	Pointer to a new driver info struct
- *		Failure:	NULL
- *
- * Programmer:	Quincey Koziol
- *		July 20 2013
+ * Purpose:     Loads an object from the disk
  *
+ * Return:      Success:    Pointer to a new driver info struct
+ *              Failure:    NULL
  *-------------------------------------------------------------------------
  */
 static void *
-H5F__cache_drvrinfo_deserialize(const void *_image, size_t H5_ATTR_NDEBUG_UNUSED len, void *_udata,
-                                hbool_t H5_ATTR_UNUSED *dirty)
+H5F__cache_drvrinfo_deserialize(const void *_image, size_t len, void *_udata, hbool_t H5_ATTR_UNUSED *dirty)
 {
     H5O_drvinfo_t           *drvinfo = NULL;                              /* Driver info */
     H5F_drvrinfo_cache_ud_t *udata   = (H5F_drvrinfo_cache_ud_t *)_udata; /* User data */
     const uint8_t           *image   = _image;                            /* Pointer into raw data buffer */
     char                     drv_name[9];                                 /* Name of driver */
-    H5O_drvinfo_t           *ret_value = NULL;                            /* Return value */
+    H5O_drvinfo_t           *ret_value = NULL;
 
     FUNC_ENTER_PACKAGE
 
-    /* Sanity check */
     HDassert(image);
     HDassert(len >= H5F_DRVINFOBLOCK_HDR_SIZE);
     HDassert(udata);
@@ -946,7 +894,7 @@ H5F__cache_drvrinfo_deserialize(const void *_image, size_t H5_ATTR_NDEBUG_UNUSED
         HGOTO_ERROR(H5E_FILE, H5E_CANTALLOC, NULL, "memory allocation failed for driver info message")
 
     /* Deserialize the file driver info's prefix */
-    if (H5F__drvrinfo_prefix_decode(drvinfo, drv_name, &image, udata, FALSE) < 0)
+    if (H5F__drvrinfo_prefix_decode(drvinfo, drv_name, &image, len, udata, FALSE) < 0)
         HGOTO_ERROR(H5E_FILE, H5E_CANTDECODE, NULL, "can't decode file driver info prefix")
 
     /* Sanity check */
@@ -959,7 +907,6 @@ H5F__cache_drvrinfo_deserialize(const void *_image, size_t H5_ATTR_NDEBUG_UNUSED
     /* Sanity check */
     HDassert((size_t)(image - (const uint8_t *)_image) <= len);
 
-    /* Set return value */
     ret_value = drvinfo;
 
 done:
@@ -973,13 +920,9 @@ done:
 /*-------------------------------------------------------------------------
  * Function:    H5F__cache_drvrinfo_image_len
  *
- * Purpose:     Compute the size of the data structure on disk.
- *
- * Return:      Non-negative on success/Negative on failure
- *
- * Programmer:  Quincey Koziol
- *              July 20, 2013
+ * Purpose:     Compute the size of the data structure on disk
  *
+ * Return:      SUCCEED/FAIL
  *-------------------------------------------------------------------------
  */
 static herr_t
@@ -989,7 +932,6 @@ H5F__cache_drvrinfo_image_len(const void *_thing, size_t *image_len)
 
     FUNC_ENTER_PACKAGE_NOERR
 
-    /* Check arguments */
     HDassert(drvinfo);
     HDassert(drvinfo->cache_info.magic == H5C__H5C_CACHE_ENTRY_T_MAGIC);
     HDassert(drvinfo->cache_info.type == H5AC_DRVRINFO);
@@ -1005,13 +947,9 @@ H5F__cache_drvrinfo_image_len(const void *_thing, size_t *image_len)
 /*-------------------------------------------------------------------------
  * Function:	H5F__cache_drvrinfo_serialize
  *
- * Purpose:	Flushes a dirty object to disk.
- *
- * Return:	Non-negative on success/Negative on failure
- *
- * Programmer:	Quincey Koziol
- *		July 20 2013
+ * Purpose:     Flush a dirty object to disk
  *
+ * Return:      SUCCEED/FAIL
  *-------------------------------------------------------------------------
  */
 static herr_t
@@ -1020,11 +958,10 @@ H5F__cache_drvrinfo_serialize(const H5F_t *f, void *_image, size_t H5_ATTR_NDEBU
     H5O_drvinfo_t *drvinfo = (H5O_drvinfo_t *)_thing; /* Pointer to the object */
     uint8_t       *image   = _image;                  /* Pointer into raw data buffer */
     uint8_t       *dbuf;                              /* Pointer to beginning of driver info */
-    herr_t         ret_value = SUCCEED;               /* Return value */
+    herr_t         ret_value = SUCCEED;
 
     FUNC_ENTER_PACKAGE
 
-    /* check arguments */
     HDassert(f);
     HDassert(image);
     HDassert(drvinfo);
@@ -1061,18 +998,14 @@ done:
 /*-------------------------------------------------------------------------
  * Function:	H5F__cache_drvrinfo_free_icr
  *
- * Purpose:	Destroy/release an "in core representation" of a data
+ * Purpose:     Destroy/release an "in core representation" of a data
  *              structure
  *
- * Note:	The metadata cache sets the object's cache_info.magic to
- *		H5C__H5C_CACHE_ENTRY_T_BAD_MAGIC before calling a free_icr
- *		callback (checked in assert).
- *
- * Return:	Non-negative on success/Negative on failure
- *
- * Programmer:	Quincey Koziol
- *              July 20, 2013
+ * Note:        The metadata cache sets the object's cache_info.magic to
+ *              H5C__H5C_CACHE_ENTRY_T_BAD_MAGIC before calling a free_icr
+ *              callback (checked in assert).
  *
+ * Return:      SUCCEED/FAIL
  *-------------------------------------------------------------------------
  */
 static herr_t
@@ -1082,7 +1015,6 @@ H5F__cache_drvrinfo_free_icr(void *_thing)
 
     FUNC_ENTER_PACKAGE_NOERR
 
-    /* Check arguments */
     HDassert(drvinfo);
     HDassert(drvinfo->cache_info.magic == H5C__H5C_CACHE_ENTRY_T_BAD_MAGIC);
     HDassert(drvinfo->cache_info.type == H5AC_DRVRINFO);