Skip to content

Commit

Permalink
Fixed asserts due to H5Pset_est_link_info() values
Browse files Browse the repository at this point in the history
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 HDFGroup#1632
  • Loading branch information
derobins committed Mar 7, 2024
1 parent fe5d0d5 commit 8162d2d
Show file tree
Hide file tree
Showing 3 changed files with 139 additions and 1 deletion.
20 changes: 20 additions & 0 deletions release_docs/RELEASE.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
20 changes: 19 additions & 1 deletion src/H5Gobj.c
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down
100 changes: 100 additions & 0 deletions test/tmisc.c
Original file line number Diff line number Diff line change
Expand Up @@ -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"

/****************************************************************
**
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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() */

Expand Down Expand Up @@ -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() */

0 comments on commit 8162d2d

Please sign in to comment.