Fixed asserts due to H5Pset_est_link_info() values

release_docs/RELEASE.txt

@@ -499,6 +499,26 @@ Bug Fixes since HDF5-1.14.0 release
 ===================================
     Library
     -------
+    - Fixed asserts raised by large values of H5Pset_est_link_info() parameters
+
+      If large values for est_num_entries and/or est_name_len were passed
+      to H5Pset_est_link_info(), the library would attempt to create an
+      object header NIL message to reserve enough space to hold the links in
+      compact form (i.e., concatenated), which could exceed allowable object
+      header message size limits and trip asserts in the library.
+
+      This bug only occurred when using the HDF5 1.8 file format or later and
+      required the product of the two values to be ~64k more than the size
+      of any links written to the group, which would cause the library to
+      write out a too-large NIL spacer message to reserve the space for the
+      unwritten links.
+
+      The library now inspects the phase change values to see if the dataset
+      is likely to be compact and checks the size to ensure any NIL spacer
+      messages won't be larger than the library allows.
+
+      Fixes GitHub #1632
+
     - Fixed a bug where H5Tset_fields does not account for any offset
       set for a floating-point datatype when determining if values set
       for spos, epos, esize, mpos and msize make sense for the datatype

src/H5Gobj.c

@@ -218,7 +218,25 @@ H5G__obj_create_real(H5F_t *f, const H5O_ginfo_t *ginfo, const H5O_linfo_t *linf
         assert(link_size);
 
         /* Compute size of header to use for creation */
-        hdr_size = linfo_size + ginfo_size + pline_size + (ginfo->est_num_entries * link_size);
+
+        /* Basic header size */
+        hdr_size = linfo_size + ginfo_size + pline_size;
+
+        /* If this is likely to be a compact group, add space for the link
+         * messages, unless the size of the link messages is greater than
+         * the largest allowable object header message size, since the size
+         * of the link messages is the size of the NIL spacer message that
+         * would have to be written out to reserve enough space to hold the
+         * links if the group were left empty.
+         */
+        bool compact = ginfo->est_num_entries <= ginfo->max_compact;
+        if (compact) {
+
+            size_t size_of_links = ginfo->est_num_entries * link_size;
+
+            if (size_of_links < H5O_MESG_MAX_SIZE)
+                hdr_size += size_of_links;
+        }
     } /* end if */
     else
         hdr_size = (size_t)(4 + 2 * H5F_SIZEOF_ADDR(f));

test/tmisc.c

@@ -338,6 +338,7 @@ typedef struct {
 #define CVE_2020_10812_FILENAME "cve_2020_10812.h5"
 
 #define MISC38_FILE "type_conversion_path_table_issue.h5"
+#define MISC39_FILE "set_est_link_info.h5"
 
 /****************************************************************
 **
@@ -6445,6 +6446,103 @@ test_misc38(void)
     CHECK(ret, FAIL, "H5Sclose");
 }
 
+/****************************************************************
+**
+**  test_misc39(): Ensure H5Pset_est_link_info() handles large
+**                 values
+**
+**  H5Pset_est_link_info() values can be set to large values,
+**  which could cause the library to attempt to create large
+**  object headers that exceed limits and trip asserts in
+**  the library.
+**
+**  This test ensures that the library doesn't error regardless
+**  of the values passed to H5Pset_est_link_info() and
+**  H5Pset_link_phase_change().
+**
+****************************************************************/
+static void
+test_misc39(void)
+{
+    hid_t  fid  = H5I_INVALID_HID; /* File ID */
+    hid_t  gid  = H5I_INVALID_HID; /* Group ID */
+    hid_t  fapl = H5I_INVALID_HID; /* File access property list ID */
+    hid_t  gcpl = H5I_INVALID_HID; /* Group creation property list ID */
+    herr_t ret  = H5I_INVALID_HID; /* Generic return value */
+
+    /* Output message about test being performed */
+    MESSAGE(5, ("Ensure H5Pset_est_link_info handles large values\n"));
+
+    /* Compose file access property list
+     *
+     * NOTE: The bug in question only occurs in new-style groups
+     */
+    fapl = H5Pcreate(H5P_FILE_ACCESS);
+    CHECK(fapl, H5I_INVALID_HID, "H5Pcreate");
+    ret = H5Pset_libver_bounds(fapl, H5F_LIBVER_LATEST, H5F_LIBVER_LATEST);
+    CHECK(ret, FAIL, "H5Pset_libver_bounds");
+
+    /* Create the file */
+    fid = H5Fcreate(MISC39_FILE, H5F_ACC_TRUNC, H5P_DEFAULT, fapl);
+    CHECK(fid, H5I_INVALID_HID, "H5Fcreate");
+
+    /* Compose group creation property list */
+    gcpl = H5Pcreate(H5P_GROUP_CREATE);
+    CHECK(gcpl, H5I_INVALID_HID, "H5Pcreate");
+
+    /* Set the estimated link info values to large numbers */
+    ret = H5Pset_est_link_info(gcpl, UINT16_MAX, UINT16_MAX);
+    CHECK(ret, FAIL, "H5Pset_est_link_info");
+
+    /* Create a group */
+    gid = H5Gcreate2(fid, "foo", H5P_DEFAULT, gcpl, H5P_DEFAULT);
+    CHECK(gid, H5I_INVALID_HID, "H5Gcreate2");
+
+    /* Close the group */
+    ret = H5Gclose(gid);
+    CHECK(ret, FAIL, "H5Gclose");
+
+    /* Close the file. Asserts typically occur here, when the metadata cache
+     * objects are flushed.
+     */
+    ret = H5Fclose(fid);
+    CHECK(ret, FAIL, "H5Fclose");
+
+    /* Re-open the file */
+    fid = H5Fopen(MISC25C_FILE, H5F_ACC_RDWR, H5P_DEFAULT);
+    CHECK(fid, H5I_INVALID_HID, "H5Fopen");
+
+    /* Set the compact/dense value high, to see if we can trick the
+     * library into creating a dense group object header that is
+     * larger than the maximum allowed size.
+     */
+    ret = H5Pset_link_phase_change(gcpl, UINT16_MAX, UINT16_MAX);
+    CHECK(ret, FAIL, "H5Pset_link_phase_change");
+
+    /* Set the estimated link info values to large numbers */
+    ret = H5Pset_est_link_info(gcpl, UINT16_MAX / 2, UINT16_MAX);
+    CHECK(ret, FAIL, "H5Pset_est_link_info");
+
+    /* Create another group */
+    gid = H5Gcreate2(fid, "bar", H5P_DEFAULT, gcpl, H5P_DEFAULT);
+    CHECK(gid, H5I_INVALID_HID, "H5Gcreate2");
+
+    /* Close the group */
+    ret = H5Gclose(gid);
+    CHECK(ret, FAIL, "H5Gclose");
+
+    /* Close the file */
+    ret = H5Fclose(fid);
+    CHECK(ret, FAIL, "H5Fclose");
+
+    /* Close the property lists */
+    ret = H5Pclose(fapl);
+    CHECK(ret, FAIL, "H5Pclose");
+    ret = H5Pclose(gcpl);
+    CHECK(ret, FAIL, "H5Pclose");
+
+} /* end test_misc25c() */
+
 /****************************************************************
 **
 **  test_misc(): Main misc. test routine.
@@ -6514,6 +6612,7 @@ test_misc(void)
     test_misc36(); /* Exercise H5atclose and H5is_library_terminating */
     test_misc37(); /* Test for seg fault failure at file close */
     test_misc38(); /* Test for type conversion path table issue */
+    test_misc39(); /* Test for type conversion path table issue */
 
 } /* test_misc() */
 
@@ -6570,6 +6669,7 @@ cleanup_misc(void)
         H5Fdelete(MISC31_FILE, H5P_DEFAULT);
 #endif /* H5_NO_DEPRECATED_SYMBOLS */
         H5Fdelete(MISC38_FILE, H5P_DEFAULT);
+        H5Fdelete(MISC39_FILE, H5P_DEFAULT);
     }
     H5E_END_TRY
 } /* end cleanup_misc() */