diff --git a/release_docs/RELEASE.txt b/release_docs/RELEASE.txt index 8695b62bba4..3896cc49456 100644 --- a/release_docs/RELEASE.txt +++ b/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 diff --git a/src/H5Gobj.c b/src/H5Gobj.c index e701922ab4e..c5bdfc1be46 100644 --- a/src/H5Gobj.c +++ b/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)); diff --git a/test/tmisc.c b/test/tmisc.c index ddebc3d648f..4f337582a1c 100644 --- a/test/tmisc.c +++ b/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_misc39() */ + /**************************************************************** ** ** 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(); /* Ensure H5Pset_est_link_info() handles large values */ } /* 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() */