Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes crashes when size_hint > UINT32_MAX is passed to H5Gcreate1 #611

Merged
merged 32 commits into from
Apr 30, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
ba9deae
Committing clang-format changes
github-actions[bot] Mar 19, 2021
00f9750
Merge remote-tracking branch 'canonical/develop' into develop
derobins Mar 21, 2021
e997283
Merge remote-tracking branch 'canonical/develop' into develop
derobins Mar 22, 2021
3eac585
Merge remote-tracking branch 'canonical/develop' into develop
derobins Mar 23, 2021
71643b7
Merge remote-tracking branch 'canonical/develop' into develop
derobins Mar 24, 2021
d242911
Merge remote-tracking branch 'canonical/develop' into develop
derobins Mar 24, 2021
bc70f95
Merge remote-tracking branch 'canonical/develop' into develop
derobins Mar 26, 2021
3d7ad8e
Merge remote-tracking branch 'canonical/develop' into develop
derobins Mar 26, 2021
aeb16b7
Merge branch 'develop' of https://github.com/derobins/hdf5 into develop
derobins Mar 26, 2021
765cb5b
Merge remote-tracking branch 'canonical/develop' into develop
derobins Mar 27, 2021
18401fc
Merge remote-tracking branch 'canonical/develop' into develop
derobins Mar 29, 2021
222242c
Merge branch 'develop' of https://github.com/derobins/hdf5 into develop
derobins Mar 29, 2021
8d0cafa
Merge remote-tracking branch 'canonical/develop' into develop
derobins Mar 30, 2021
d3694d1
Merge remote-tracking branch 'canonical/develop' into develop
derobins Apr 2, 2021
1c7bcc2
Merge remote-tracking branch 'canonical/develop' into develop
derobins Apr 15, 2021
70551e3
Merge remote-tracking branch 'canonical/develop' into develop
derobins Apr 16, 2021
2dfc7db
Merge remote-tracking branch 'canonical/develop' into develop
derobins Apr 20, 2021
71d2fbf
Merge remote-tracking branch 'canonical/develop' into develop
derobins Apr 21, 2021
c8925f8
Merge remote-tracking branch 'canonical/develop' into develop
derobins Apr 22, 2021
dd3e970
Merge branch 'develop' of https://github.com/derobins/hdf5 into develop
derobins Apr 22, 2021
bb61990
Merge remote-tracking branch 'canonical/develop' into develop
derobins Apr 23, 2021
2c2b368
Merge remote-tracking branch 'canonical/develop' into develop
derobins Apr 25, 2021
be2ada6
Merge remote-tracking branch 'canonical/develop' into develop
derobins Apr 27, 2021
439f3a8
Merge remote-tracking branch 'canonical/develop' into develop
derobins Apr 28, 2021
649aa9b
Merge remote-tracking branch 'canonical/develop' into develop
derobins Apr 29, 2021
3fa627f
Merge remote-tracking branch 'canonical/develop' into develop
derobins Apr 29, 2021
7a6ee4e
Fixes incorrect size_hint handling in H5Gcreate1
derobins Apr 30, 2021
17fc91e
Updates the size hint type for group creation
derobins Apr 30, 2021
3df386a
Updates the RELEASE.txt note
derobins Apr 30, 2021
5f825f9
Revert "Updates the RELEASE.txt note"
derobins Apr 30, 2021
850e79e
Reverts previous behavior to use a uint32_t struct field
derobins Apr 30, 2021
a4227fb
Updates RELEASE.txt
derobins Apr 30, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions release_docs/RELEASE.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
10 changes: 6 additions & 4 deletions src/H5Gdeprec.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
*
Expand Down Expand Up @@ -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) {
Expand Down
12 changes: 5 additions & 7 deletions src/H5Gpublic.h
Original file line number Diff line number Diff line change
Expand Up @@ -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}
*
Expand All @@ -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
Expand Down
25 changes: 23 additions & 2 deletions test/tmisc.c
Original file line number Diff line number Diff line change
Expand Up @@ -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");

Expand All @@ -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 */
Expand Down