mirrors/hdf5
2
0
Fork 0
mirror of https://github.com/HDFGroup/hdf5.git synced 2026-09-25 04:09:44 +03:00
Code Issues Packages Projects Releases Wiki Activity

Detect invalid ID to H5Gmove2 (#4765)

Browse Source 
User's application segfaulted because the returned value H5I_BADID wasn't
detected when H5I_get_type() was called.  This PR adds checks for invalid
file/group identifiers passed into H5Gmove2.

This defect occurs in many other places, hence, issue GH-4764.

Fixes #4737
This commit is contained in:
bmribler
2024-09-06 08:50:10 -07:00
committed by GitHub
parent 2eaa01690a
commit a75542b3e7
4 changed files with 135 additions and 39 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/H5Gdeprec.c
+33 -10
View File
@@ -519,10 +519,35 @@ H5Gmove2(hid_t src_loc_id, const char *src_name, hid_t dst_loc_id, const char *d
H5VL_loc_params_t loc_params1;
H5VL_object_t *vol_obj2 = NULL; /* Object of dst_id */
H5VL_loc_params_t loc_params2;
H5I_type_t src_id_type = H5I_BADID, dst_id_type = H5I_BADID;
herr_t ret_value = SUCCEED; /* Return value */
FUNC_ENTER_API(FAIL)
/* Check arguments */
if (!src_name || !*src_name)
HGOTO_ERROR(H5E_ARGS, H5E_BADVALUE, FAIL, "no current name specified");
if (!dst_name || !*dst_name)
HGOTO_ERROR(H5E_ARGS, H5E_BADVALUE, FAIL, "no destination name specified");
/* src and dst location IDs cannot both have the value of H5L_SAME_LOC */
if (src_loc_id == H5L_SAME_LOC && dst_loc_id == H5L_SAME_LOC)
HGOTO_ERROR(H5E_ARGS, H5E_BADVALUE, FAIL, "current and destination should not both be H5L_SAME_LOC");
/* reset an ID in the case of H5L_SAME_LOC */
if (src_loc_id == H5L_SAME_LOC)
src_loc_id = dst_loc_id;
else if (dst_loc_id == H5L_SAME_LOC)
dst_loc_id = src_loc_id;
src_id_type = H5I_get_type(src_loc_id);
if (!(H5I_GROUP == src_id_type || H5I_FILE == src_id_type))
HGOTO_ERROR(H5E_ARGS, H5E_BADVALUE, FAIL, "invalid group (or file) ID, src_loc_id");
dst_id_type = H5I_get_type(dst_loc_id);
if (!(H5I_GROUP == dst_id_type || H5I_FILE == dst_id_type))
HGOTO_ERROR(H5E_ARGS, H5E_BADVALUE, FAIL, "invalid group (or file) ID, dst_loc_id");
/* Set up collective metadata if appropriate */
if (H5CX_set_loc(dst_loc_id) < 0)
HGOTO_ERROR(H5E_SYM, H5E_CANTSET, FAIL, "can't set collective metadata read info");
@@ -531,22 +556,20 @@ H5Gmove2(hid_t src_loc_id, const char *src_name, hid_t dst_loc_id, const char *d
loc_params1.type = H5VL_OBJECT_BY_NAME;
loc_params1.loc_data.loc_by_name.name = src_name;
loc_params1.loc_data.loc_by_name.lapl_id = H5P_LINK_ACCESS_DEFAULT;
loc_params1.obj_type = H5I_get_type(src_loc_id);
loc_params1.obj_type = src_id_type;
/* Set location parameter for destination object */
loc_params2.type = H5VL_OBJECT_BY_NAME;
loc_params2.loc_data.loc_by_name.name = dst_name;
loc_params2.loc_data.loc_by_name.lapl_id = H5P_LINK_ACCESS_DEFAULT;
loc_params2.obj_type = H5I_get_type(dst_loc_id);
loc_params2.obj_type = dst_id_type;
if (H5L_SAME_LOC != src_loc_id)
/* get the location object */
if (NULL == (vol_obj1 = H5VL_vol_object(src_loc_id)))
HGOTO_ERROR(H5E_ARGS, H5E_BADTYPE, FAIL, "invalid location identifier");
if (H5L_SAME_LOC != dst_loc_id)
/* get the location object */
if (NULL == (vol_obj2 = H5VL_vol_object(dst_loc_id)))
HGOTO_ERROR(H5E_ARGS, H5E_BADTYPE, FAIL, "invalid location identifier");
/* get the location object */
if (NULL == (vol_obj1 = H5VL_vol_object(src_loc_id)))
HGOTO_ERROR(H5E_ARGS, H5E_BADTYPE, FAIL, "invalid location identifier");
/* get the location object */
if (NULL == (vol_obj2 = H5VL_vol_object(dst_loc_id)))
HGOTO_ERROR(H5E_ARGS, H5E_BADTYPE, FAIL, "invalid location identifier");
/* Move the link */
if (H5VL_link_move(vol_obj1, &loc_params1, vol_obj2, &loc_params2, H5P_LINK_CREATE_DEFAULT,
src/H5L.c
+50 -24
View File
@@ -94,7 +94,8 @@ H5Lmove(hid_t src_loc_id, const char *src_name, hid_t dst_loc_id, const char *ds
H5VL_object_t *vol_obj2 = NULL; /* Object of dst_id */
H5VL_loc_params_t loc_params1;
H5VL_loc_params_t loc_params2;
H5VL_object_t tmp_vol_obj; /* Temporary object */
H5VL_object_t tmp_vol_obj; /* Temporary object */
H5I_type_t src_id_type = H5I_BADID, dst_id_type = H5I_BADID;
herr_t ret_value = SUCCEED; /* Return value */
FUNC_ENTER_API(FAIL)
@@ -106,6 +107,21 @@ H5Lmove(hid_t src_loc_id, const char *src_name, hid_t dst_loc_id, const char *ds
HGOTO_ERROR(H5E_ARGS, H5E_BADVALUE, FAIL, "no current name specified");
if (!dst_name || !*dst_name)
HGOTO_ERROR(H5E_ARGS, H5E_BADVALUE, FAIL, "no destination name specified");
/* reset an ID in the case of H5L_SAME_LOC */
if (src_loc_id == H5L_SAME_LOC)
src_loc_id = dst_loc_id;
else if (dst_loc_id == H5L_SAME_LOC)
dst_loc_id = src_loc_id;
/* verify that src and dst IDs are either a file or a group ID */
src_id_type = H5I_get_type(src_loc_id);
if (!(H5I_GROUP == src_id_type || H5I_FILE == src_id_type))
HGOTO_ERROR(H5E_ARGS, H5E_BADVALUE, FAIL, "invalid group (or file) ID, src_loc_id");
dst_id_type = H5I_get_type(dst_loc_id);
if (!(H5I_GROUP == dst_id_type || H5I_FILE == dst_id_type))
HGOTO_ERROR(H5E_ARGS, H5E_BADVALUE, FAIL, "invalid group (or file) ID, dst_loc_id");
if (lcpl_id != H5P_DEFAULT && (true != H5P_isa_class(lcpl_id, H5P_LINK_CREATE)))
HGOTO_ERROR(H5E_ARGS, H5E_BADTYPE, FAIL, "not a link creation property list");
@@ -117,30 +133,27 @@ H5Lmove(hid_t src_loc_id, const char *src_name, hid_t dst_loc_id, const char *ds
H5CX_set_lcpl(lcpl_id);
/* Verify access property list and set up collective metadata if appropriate */
if (H5CX_set_apl(&lapl_id, H5P_CLS_LACC, ((src_loc_id != H5L_SAME_LOC) ? src_loc_id : dst_loc_id), true) <
0)
if (H5CX_set_apl(&lapl_id, H5P_CLS_LACC, dst_loc_id, true) < 0)
HGOTO_ERROR(H5E_LINK, H5E_CANTSET, FAIL, "can't set access property list info");
/* Set location parameter for source object */
loc_params1.type = H5VL_OBJECT_BY_NAME;
loc_params1.loc_data.loc_by_name.name = src_name;
loc_params1.loc_data.loc_by_name.lapl_id = lapl_id;
loc_params1.obj_type = H5I_get_type(src_loc_id);
loc_params1.obj_type = src_id_type;
/* Set location parameter for destination object */
loc_params2.type = H5VL_OBJECT_BY_NAME;
loc_params2.loc_data.loc_by_name.name = dst_name;
loc_params2.loc_data.loc_by_name.lapl_id = lapl_id;
loc_params2.obj_type = H5I_get_type(dst_loc_id);
loc_params2.obj_type = dst_id_type;
if (H5L_SAME_LOC != src_loc_id)
/* Get the location object */
if (NULL == (vol_obj1 = H5VL_vol_object(src_loc_id)))
HGOTO_ERROR(H5E_ARGS, H5E_BADTYPE, FAIL, "invalid location identifier");
if (H5L_SAME_LOC != dst_loc_id)
/* Get the location object */
if (NULL == (vol_obj2 = H5VL_vol_object(dst_loc_id)))
HGOTO_ERROR(H5E_ARGS, H5E_BADTYPE, FAIL, "invalid location identifier");
/* Get the location object */
if (NULL == (vol_obj1 = H5VL_vol_object(src_loc_id)))
HGOTO_ERROR(H5E_ARGS, H5E_BADTYPE, FAIL, "invalid location identifier");
/* Get the location object */
if (NULL == (vol_obj2 = H5VL_vol_object(dst_loc_id)))
HGOTO_ERROR(H5E_ARGS, H5E_BADTYPE, FAIL, "invalid location identifier");
/* Make sure that the VOL connectors are the same */
if (vol_obj1 && vol_obj2) {
@@ -195,7 +208,8 @@ H5Lcopy(hid_t src_loc_id, const char *src_name, hid_t dst_loc_id, const char *ds
H5VL_loc_params_t loc_params1;
H5VL_object_t *vol_obj2 = NULL; /* Object of dst_id */
H5VL_loc_params_t loc_params2;
H5VL_object_t tmp_vol_obj; /* Temporary object */
H5VL_object_t tmp_vol_obj; /* Temporary object */
H5I_type_t src_id_type = H5I_BADID, dst_id_type = H5I_BADID;
herr_t ret_value = SUCCEED; /* Return value */
FUNC_ENTER_API(FAIL)
@@ -210,6 +224,20 @@ H5Lcopy(hid_t src_loc_id, const char *src_name, hid_t dst_loc_id, const char *ds
if (lcpl_id != H5P_DEFAULT && (true != H5P_isa_class(lcpl_id, H5P_LINK_CREATE)))
HGOTO_ERROR(H5E_ARGS, H5E_BADTYPE, FAIL, "not a link creation property list");
/* reset an ID in the case of H5L_SAME_LOC */
if (src_loc_id == H5L_SAME_LOC)
src_loc_id = dst_loc_id;
else if (dst_loc_id == H5L_SAME_LOC)
dst_loc_id = src_loc_id;
/* verify that src and dst IDs are either a file or a group ID */
src_id_type = H5I_get_type(src_loc_id);
if (!(H5I_GROUP == src_id_type || H5I_FILE == src_id_type) && src_loc_id != H5L_SAME_LOC)
HGOTO_ERROR(H5E_ARGS, H5E_BADVALUE, FAIL, "invalid group (or file) ID, src_loc_id");
dst_id_type = H5I_get_type(dst_loc_id);
if (!(H5I_GROUP == dst_id_type || H5I_FILE == dst_id_type) && dst_loc_id != H5L_SAME_LOC)
HGOTO_ERROR(H5E_ARGS, H5E_BADVALUE, FAIL, "invalid group (or file) ID, dst_loc_id");
/* Check the link create property list */
if (H5P_DEFAULT == lcpl_id)
lcpl_id = H5P_LINK_CREATE_DEFAULT;
@@ -226,22 +254,20 @@ H5Lcopy(hid_t src_loc_id, const char *src_name, hid_t dst_loc_id, const char *ds
loc_params1.type = H5VL_OBJECT_BY_NAME;
loc_params1.loc_data.loc_by_name.name = src_name;
loc_params1.loc_data.loc_by_name.lapl_id = lapl_id;
loc_params1.obj_type = H5I_get_type(src_loc_id);
loc_params1.obj_type = src_id_type;
/* Set location parameter for destination object */
loc_params2.type = H5VL_OBJECT_BY_NAME;
loc_params2.loc_data.loc_by_name.name = dst_name;
loc_params2.loc_data.loc_by_name.lapl_id = lapl_id;
loc_params2.obj_type = H5I_get_type(dst_loc_id);
loc_params2.obj_type = dst_id_type;
if (H5L_SAME_LOC != src_loc_id)
/* Get the location object */
if (NULL == (vol_obj1 = H5VL_vol_object(src_loc_id)))
HGOTO_ERROR(H5E_ARGS, H5E_BADTYPE, FAIL, "invalid location identifier");
if (H5L_SAME_LOC != dst_loc_id)
/* Get the location object */
if (NULL == (vol_obj2 = H5VL_vol_object(dst_loc_id)))
HGOTO_ERROR(H5E_ARGS, H5E_BADTYPE, FAIL, "invalid location identifier");
/* Get the location object */
if (NULL == (vol_obj1 = H5VL_vol_object(src_loc_id)))
HGOTO_ERROR(H5E_ARGS, H5E_BADTYPE, FAIL, "invalid location identifier");
/* Get the location object */
if (NULL == (vol_obj2 = H5VL_vol_object(dst_loc_id)))
HGOTO_ERROR(H5E_ARGS, H5E_BADTYPE, FAIL, "invalid location identifier");
/* Make sure that the VOL connectors are the same */
if (vol_obj1 && vol_obj2) {
src/H5VLcallback.c
+4
View File
@@ -5100,6 +5100,10 @@ H5VL_link_move(const H5VL_object_t *src_vol_obj, const H5VL_loc_params_t *loc_pa
FUNC_ENTER_NOAPI(FAIL)
/* Sanity check */
assert(src_vol_obj);
assert(src_vol_obj->data);
/* Set wrapper info in API context */
vol_obj = (src_vol_obj->data ? src_vol_obj : dst_vol_obj);
if (H5VL_set_vol_wrapper(vol_obj) < 0)
test/links.c
+48 -5
View File
@@ -1938,12 +1938,14 @@ error:
*-------------------------------------------------------------------------
*/
#ifndef H5_NO_DEPRECATED_SYMBOLS
#define NUM_OBJS 3 /* number of groups in FILENAME[0] file */
static int
test_deprec(hid_t fapl, bool new_format)
{
hid_t file_id = H5I_INVALID_HID;
hid_t group1_id = H5I_INVALID_HID;
hid_t group2_id = H5I_INVALID_HID;
hid_t group3_id = H5I_INVALID_HID;
H5G_stat_t sb_hard1, sb_hard2, sb_soft1, sb_soft2;
H5G_obj_t obj_type; /* Object type */
hsize_t num_objs; /* Number of objects in a group */
@@ -1967,6 +1969,8 @@ test_deprec(hid_t fapl, bool new_format)
FAIL_STACK_ERROR;
if ((group2_id = H5Gcreate2(file_id, "group2", H5P_DEFAULT, H5P_DEFAULT, H5P_DEFAULT)) < 0)
FAIL_STACK_ERROR;
if ((group3_id = H5Gcreate2(file_id, "group3", H5P_DEFAULT, H5P_DEFAULT, H5P_DEFAULT)) < 0)
FAIL_STACK_ERROR;
/* Test H5Gset and get comment */
@@ -2022,7 +2026,7 @@ test_deprec(hid_t fapl, bool new_format)
/* Test getting the number of objects in a group */
if (H5Gget_num_objs(file_id, &num_objs) < 0)
FAIL_STACK_ERROR;
if (num_objs != 2)
if (num_objs != NUM_OBJS)
TEST_ERROR;
if (H5Gget_num_objs(group1_id, &num_objs) < 0)
FAIL_STACK_ERROR;
@@ -2113,9 +2117,43 @@ test_deprec(hid_t fapl, bool new_format)
/* Test H5Gmove and H5Gmove2 */
if (H5Gmove(file_id, "group1", "moved_group1") < 0)
FAIL_STACK_ERROR;
TEST_ERROR;
if (H5Gmove2(file_id, "group2", group1_id, "moved_group2") < 0)
FAIL_STACK_ERROR;
TEST_ERROR;
if (H5Gmove2(file_id, "group3", H5L_SAME_LOC, "moved_group3") < 0)
TEST_ERROR;
if (H5Gmove2(file_id, "moved_group3", group2_id, "moved_group3_to_group2") < 0)
TEST_ERROR;
/* Test H5Gmove2 with H5L_SAME_LOC */
if (H5Gmove2(group2_id, "moved_group3_to_group2", H5L_SAME_LOC, "group3_same_loc") < 0)
TEST_ERROR;
/* Test H5Gmove2 with H5L_SAME_LOC */
if (H5Gmove2(H5L_SAME_LOC, "moved_group1/moved_group2", file_id, "moved_group2_again") < 0)
TEST_ERROR;
/* Put back moved_group2 for subsequent tests */
if (H5Gmove2(file_id, "moved_group2_again", file_id, "moved_group1/moved_group2") < 0)
TEST_ERROR;
/* Test passing in invalid ID */
H5E_BEGIN_TRY
{
hid_t bad_id = H5I_BADID;
if (H5Gmove2(bad_id, "group2", group1_id, "moved_group2") >= 0)
TEST_ERROR;
}
H5E_END_TRY
/* Test passing in invalid ID */
H5E_BEGIN_TRY
{
hid_t bad_id = H5I_BADID;
if (H5Gmove2(file_id, "group2", bad_id, "moved_group2") >= 0)
TEST_ERROR;
}
H5E_END_TRY
/* Ensure that both groups can be opened */
if (H5Gclose(group2_id) < 0)
@@ -2129,6 +2167,8 @@ test_deprec(hid_t fapl, bool new_format)
FAIL_STACK_ERROR;
/* Close open IDs */
if (H5Gclose(group3_id) < 0)
FAIL_STACK_ERROR;
if (H5Gclose(group2_id) < 0)
FAIL_STACK_ERROR;
if (H5Gclose(group1_id) < 0)
@@ -2154,6 +2194,7 @@ test_deprec(hid_t fapl, bool new_format)
error:
H5E_BEGIN_TRY
{
H5Gclose(group3_id);
H5Gclose(group2_id);
H5Gclose(group1_id);
H5Fclose(file_id);
@@ -3293,7 +3334,8 @@ external_link_closing_deprec(hid_t fapl, bool new_format)
/* Test copy (as of this test, it uses the same code as move) */
if (H5Lcopy(fid1, "elink/elink/elink", fid1, "elink/elink/elink_copied", H5P_DEFAULT, H5P_DEFAULT) < 0)
FAIL_STACK_ERROR;
if (H5Lcopy(fid1, "elink/elink/elink", fid1, "elink/elink/elink/elink_copied2", H5P_DEFAULT,
/* Also exercise H5L_SAME_LOC */
if (H5Lcopy(H5L_SAME_LOC, "elink/elink/elink", fid1, "elink/elink/elink/elink_copied2", H5P_DEFAULT,
H5P_DEFAULT) < 0)
FAIL_STACK_ERROR;
@@ -4325,7 +4367,8 @@ lapl_nlinks_deprec(hid_t fapl, bool new_format)
*/
if (H5Lcopy(fid, "soft17", fid, "soft17/newer_soft", H5P_DEFAULT, plist) < 0)
TEST_ERROR;
if (H5Lmove(fid, "soft17/newer_soft", fid, "soft17/newest_soft", H5P_DEFAULT, plist) < 0)
/* Also exercise H5L_SAME_LOC */
if (H5Lmove(fid, "soft17/newer_soft", H5L_SAME_LOC, "soft17/newest_soft", H5P_DEFAULT, plist) < 0)
TEST_ERROR;
/* H5Olink */