summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorDana Robinson <43805+derobins@users.noreply.github.com>2021-04-30 19:47:51 (GMT)
committerGitHub <noreply@github.com>2021-04-30 19:47:51 (GMT)
commit73bb382e9e77ca9847c09dff37b9e2338f26bbb8 (patch)
tree85e726e1aae576dc13ac76da36c92305c669ec92
parent7ab97037a0f64a583a6c7d4064d92a963cdfd843 (diff)
downloadhdf5-73bb382e9e77ca9847c09dff37b9e2338f26bbb8.zip
hdf5-73bb382e9e77ca9847c09dff37b9e2338f26bbb8.tar.gz
hdf5-73bb382e9e77ca9847c09dff37b9e2338f26bbb8.tar.bz2
Fixes crashes when size_hint > UINT32_MAX is passed to H5Gcreate1 (#611)
* Committing clang-format changes * Fixes incorrect size_hint handling in H5Gcreate1 * Updates the size hint type for group creation * Updates the RELEASE.txt note * Revert "Updates the RELEASE.txt note" This reverts commit 3df386acca806d652bbe2209f7c4503b30f068ff. * Reverts previous behavior to use a uint32_t struct field * Updates RELEASE.txt Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>
-rw-r--r--release_docs/RELEASE.txt18
-rw-r--r--src/H5Gdeprec.c10
-rw-r--r--src/H5Gpublic.h12
-rw-r--r--test/tmisc.c25
4 files changed, 52 insertions, 13 deletions
diff --git a/release_docs/RELEASE.txt b/release_docs/RELEASE.txt
index 8b07393..be8440f 100644
--- a/release_docs/RELEASE.txt
+++ b/release_docs/RELEASE.txt
@@ -414,6 +414,24 @@ New Features
Library:
--------
+ - H5Gcreate1() now rejects size_hint parameters larger than UINT32_MAX
+
+ The size_hint value is ultimately stored in a uint32_t struct field,
+ so specifying a value larger than this on a 64-bit machine can cause
+ undefined behavior including crashing the system.
+
+ The documentation for this API call was also incorrect, stating that
+ passing a negative value would cause the library to use a default
+ value. Instead, passing a "negative" value actually passes a very large
+ value, which is probably not what the user intends and can cause
+ crashes on 64-bit systems.
+
+ The Doxygen documentation has been updated and passing values larger
+ than UINT32_MAX for size_hint will now produce a normal HDF5 error.
+
+ (DER - 2021/04/29, HDFFV-11241)
+
+
- H5Pset_fapl_log() no longer crashes when passed an invalid fapl ID
When passed an invalid fapl ID, H5Pset_fapl_log() would usually
diff --git a/src/H5Gdeprec.c b/src/H5Gdeprec.c
index 453796e..45065e8 100644
--- a/src/H5Gdeprec.c
+++ b/src/H5Gdeprec.c
@@ -141,10 +141,10 @@ H5G_map_obj_type(H5O_type_t obj_type)
* specified NAME. The group is opened for write access
* and it's object ID is returned.
*
- * The optional SIZE_HINT specifies how much file space to
- * reserve to store the names that will appear in this
- * group. If a non-positive value is supplied for the SIZE_HINT
- * then a default size is chosen.
+ * The SIZE_HINT parameter specifies how much file space to reserve
+ * to store the names that will appear in this group. This number
+ * must be less than or equal to UINT32_MAX. If zero is supplied
+ * for the SIZE_HINT then a default size is chosen.
*
* Note: Deprecated in favor of H5Gcreate2
*
@@ -174,6 +174,8 @@ H5Gcreate1(hid_t loc_id, const char *name, size_t size_hint)
/* Check arguments */
if (!name || !*name)
HGOTO_ERROR(H5E_ARGS, H5E_BADVALUE, H5I_INVALID_HID, "no name given")
+ if (size_hint > UINT32_MAX)
+ HGOTO_ERROR(H5E_ARGS, H5E_BADVALUE, H5I_INVALID_HID, "size_hint cannot be larger than UINT32_MAX")
/* Check if we need to create a non-standard GCPL */
if (size_hint > 0) {
diff --git a/src/H5Gpublic.h b/src/H5Gpublic.h
index 416ff2c..b009d41 100644
--- a/src/H5Gpublic.h
+++ b/src/H5Gpublic.h
@@ -569,8 +569,8 @@ typedef struct H5G_stat_t {
*
* \fgdta_loc_id
* \param[in] name Name of the group to create
- * \param[in] size_hint Optional parameter indicating the number of bytes
- * to reserve for the names that will appear in the group
+ * \param[in] size_hint The number of bytes to reserve for the names
+ * that will appear in the group
*
* \return \hid_t{group}
*
@@ -592,11 +592,9 @@ typedef struct H5G_stat_t {
* group, is not limited.
*
* \p size_hint is a hint for the number of bytes to reserve to store
- * the names which will be eventually added to the new group. Passing a
- * value of zero for \p size_hint is usually adequate since the library
- * is able to dynamically resize the name heap, but a correct hint may
- * result in better performance. If a non-positive value is supplied
- * for \p size_hint, then a default size is chosen.
+ * the names which will be eventually added to the new group. This
+ * value must be between 0 and UINT32_MAX (inclusive). If this
+ * parameter is zero, a default value will be used.
*
* The return value is a group identifier for the open group. This
* group identifier should be closed by calling H5Gclose() when it is
diff --git a/test/tmisc.c b/test/tmisc.c
index 03f9952..51e203a 100644
--- a/test/tmisc.c
+++ b/test/tmisc.c
@@ -4088,9 +4088,31 @@ test_misc23(void)
H5E_END_TRY;
VERIFY(tmp_id, FAIL, "H5Gcreate1");
- tmp_id = H5Gcreate1(file_id, "/A/grp", (size_t)0);
+ /* Make sure that size_hint values that can't fit into a 32-bit
+ * unsigned integer are rejected. Only necessary on systems where
+ * size_t is a 64-bit type.
+ */
+ if (SIZE_MAX > UINT32_MAX) {
+ H5E_BEGIN_TRY
+ {
+ tmp_id = H5Gcreate1(file_id, "/size_hint_too_large", SIZE_MAX);
+ }
+ H5E_END_TRY;
+ VERIFY(tmp_id, FAIL, "H5Gcreate1");
+ }
+
+ /* Make sure the largest size_hint value works */
+ H5E_BEGIN_TRY
+ {
+ tmp_id = H5Gcreate1(file_id, "/largest_size_hint", UINT32_MAX);
+ }
+ H5E_END_TRY;
CHECK(tmp_id, FAIL, "H5Gcreate1");
+ status = H5Gclose(tmp_id);
+ CHECK(status, FAIL, "H5Gclose");
+ tmp_id = H5Gcreate1(file_id, "/A/grp", (size_t)0);
+ CHECK(tmp_id, FAIL, "H5Gcreate1");
status = H5Gclose(tmp_id);
CHECK(status, FAIL, "H5Gclose");
@@ -4103,7 +4125,6 @@ test_misc23(void)
tmp_id = H5Dcreate1(file_id, "/A/dset", type_id, space_id, create_id);
CHECK(tmp_id, FAIL, "H5Dcreate1");
-
status = H5Dclose(tmp_id);
CHECK(status, FAIL, "H5Dclose");
#endif /* H5_NO_DEPRECATED_SYMBOLS */