From ee4d92e01bd7a48624af7a8a811faea00724daff Mon Sep 17 00:00:00 2001 From: raylu-hdf <60487644+raylu-hdf@users.noreply.github.com> Date: Thu, 17 Nov 2022 16:29:21 -0600 Subject: Bug Fix: Segfault in H5CX_get_vol_wrap_ctx when H5VLwrap_register is called from an application (#2248) * Jira HDFFV-10881: Segfault in H5CX_get_vol_wrap_ctx when H5VLwrap_register is called from an application. A quick and simple fix to make it fail with a relevant error message. * Format changes. * Committing clang-format changes * Minor change: split one condition check into two for clarity. * Adding doxygen comment for H5VLwrap_register. * Minor change: adding a little more detail to the Doxygen comment for H5VLwrap_register. --- src/H5CX.c | 13 ++++++-- src/H5VLconnector_passthru.h | 24 ++++++++++++++ test/vol.c | 76 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 111 insertions(+), 2 deletions(-) diff --git a/src/H5CX.c b/src/H5CX.c index 24f10d9..95e824d 100644 --- a/src/H5CX.c +++ b/src/H5CX.c @@ -1621,12 +1621,20 @@ H5CX_get_vol_wrap_ctx(void **vol_wrap_ctx) H5CX_node_t **head = NULL; /* Pointer to head of API context list */ herr_t ret_value = SUCCEED; /* Return value */ - FUNC_ENTER_NOAPI_NOERR + FUNC_ENTER_NOAPI(FAIL) /* Sanity check */ HDassert(vol_wrap_ctx); head = H5CX_get_my_context(); /* Get the pointer to the head of the API context, for this thread */ - HDassert(head && *head); + + /* No error is expected at this point. But in case an application calls H5VLwrap_register + * which doesn't reset the API context and there is no context, returns a relevant error here + */ + if (!head) + HGOTO_ERROR(H5E_CONTEXT, H5E_UNINITIALIZED, FAIL, "the API context isn't available") + + if (!(*head)) + HGOTO_ERROR(H5E_CONTEXT, H5E_CANTGET, FAIL, "unable to get the current API context") /* Check for value that was set */ if ((*head)->ctx.vol_wrap_ctx_valid) @@ -1635,6 +1643,7 @@ H5CX_get_vol_wrap_ctx(void **vol_wrap_ctx) else *vol_wrap_ctx = NULL; +done: FUNC_LEAVE_NOAPI(ret_value) } /* end H5CX_get_vol_wrap_ctx() */ diff --git a/src/H5VLconnector_passthru.h b/src/H5VLconnector_passthru.h index 079a869..68dd33a 100644 --- a/src/H5VLconnector_passthru.h +++ b/src/H5VLconnector_passthru.h @@ -56,6 +56,30 @@ extern "C" { /* Helper routines for VOL connector authors */ H5_DLL herr_t H5VLcmp_connector_cls(int *cmp, hid_t connector_id1, hid_t connector_id2); +/** + * \ingroup H5VL + * + * \brief Wrap an internal object with a "wrap context" and register an + * hid_t for the resulting object. + * + * \param[in] obj VOL object. + * \param[in] type VOL-managed object class. Allowable values are: + * - #H5I_FILE + * - #H5I_GROUP + * - #H5I_DATATYPE + * - #H5I_DATASET + * - #H5I_MAP + * - #H5I_ATTR + * + * \return \hid_t + * + * \note This routine is mainly targeted toward wrapping objects for + * iteration routine callbacks (i.e. the callbacks from H5Aiterate*, + * H5Literate* / H5Lvisit*, and H5Ovisit* ). Using it in an application + * will return an error indicating the API context isn't available or + * can't be retrieved. + * + */ H5_DLL hid_t H5VLwrap_register(void *obj, H5I_type_t type); H5_DLL herr_t H5VLretrieve_lib_state(void **state); H5_DLL herr_t H5VLstart_lib_state(void); diff --git a/test/vol.c b/test/vol.c index 7028497..d369669 100644 --- a/test/vol.c +++ b/test/vol.c @@ -2290,6 +2290,81 @@ error: } /* end test_vol_cap_flags() */ /*------------------------------------------------------------------------- + * Function: test_wrap_register() + * + * Purpose: Tests the failure of calling H5VLwrap_register in application + * + * Return: SUCCEED/FAIL + * + *------------------------------------------------------------------------- + */ +static herr_t +test_wrap_register(void) +{ + hid_t fapl_id = H5I_INVALID_HID; + hid_t file_id = H5I_INVALID_HID; + hid_t group_id = H5I_INVALID_HID; + hid_t wrap_id = H5I_INVALID_HID; + char filename[NAME_LEN]; + void *vol_obj; + + TESTING("failure of calling H5VLwrap_register"); + + /* Retrieve the file access property for testing */ + fapl_id = h5_fileaccess(); + + h5_fixname(FILENAME[0], fapl_id, filename, sizeof filename); + + if ((file_id = H5Fcreate(filename, H5F_ACC_TRUNC, H5P_DEFAULT, fapl_id)) < 0) + TEST_ERROR; + + if ((group_id = H5Gcreate2(file_id, "test", H5P_DEFAULT, H5P_DEFAULT, H5P_DEFAULT)) < 0) + TEST_ERROR; + + /* Retrieve the VOL object for the group */ + if (!(vol_obj = H5VLobject(group_id))) + TEST_ERROR; + + /* Try to register a second ID for the group. It should fail because this routine is mainly + * targeted toward wrapping objects for iteration routine callbacks, not for application use + */ + H5E_BEGIN_TRY + { + wrap_id = H5VLwrap_register(vol_obj, H5I_GROUP); + } + H5E_END_TRY; + + if (H5I_INVALID_HID != wrap_id) + FAIL_PUTS_ERROR("should not be able to call H5VLwrap_register in an application"); + + if (H5Gclose(group_id) < 0) + TEST_ERROR; + + if (H5Fclose(file_id) < 0) + TEST_ERROR; + + h5_delete_test_file(FILENAME[0], fapl_id); + + if (H5Pclose(fapl_id) < 0) + TEST_ERROR; + + PASSED(); + + return SUCCEED; + +error: + H5E_BEGIN_TRY + { + H5Gclose(group_id); + H5Fclose(file_id); + H5Pclose(fapl_id); + } + H5E_END_TRY; + + return FAIL; +} /* end test_wrap_register() */ + +/*------------------------------------------------------------------------- * Function: main * * Purpose: Tests the virtual object layer interface (H5VL) @@ -2326,6 +2401,7 @@ main(void) nerrors += test_async_vol_props() < 0 ? 1 : 0; nerrors += test_vol_cap_flags() < 0 ? 1 : 0; nerrors += test_get_vol_name() < 0 ? 1 : 0; + nerrors += test_wrap_register() < 0 ? 1 : 0; if (nerrors) { HDprintf("***** %d Virtual Object Layer TEST%s FAILED! *****\n", nerrors, nerrors > 1 ? "S" : ""); -- cgit v0.12