From 16f9b070ce084313de37cc73968f70f2cba0b75a Mon Sep 17 00:00:00 2001 From: Dana Robinson <43805+derobins@users.noreply.github.com> Date: Thu, 29 Apr 2021 07:33:43 -0700 Subject: Fixes a segfault when H5Pset_mdc_log_options is called multiple times on a fapl (#601) * Committing clang-format changes * Fixes a segfault when H5Pset_mdc_log_options() is called multiple times An internal string is incorrectly freed when the API call is invoked multiple times on a property list, which will usually cause a segfault to occur. On the first call the log location is NULL so the problem doesn't occur. Fixes HDFFV-11239 * Fixes typos Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com> --- release_docs/RELEASE.txt | 12 ++++++++++++ src/H5Pfapl.c | 11 +++-------- test/cache_logging.c | 6 ++++++ 3 files changed, 21 insertions(+), 8 deletions(-) diff --git a/release_docs/RELEASE.txt b/release_docs/RELEASE.txt index f014d72..8b07393 100644 --- a/release_docs/RELEASE.txt +++ b/release_docs/RELEASE.txt @@ -427,6 +427,18 @@ New Features (DER - 2021/04/28, HDFFV-11240) + - Fixes a segfault when H5Pset_mdc_log_options() is called multiple times + + The call incorrectly attempts to free an internal copy of the previous + log location string, which causes a segfault. This only happens + when the call is invoked multiple times on the same property list. + On the first call to a given fapl, the log location is set to NULL so + the segfault does not occur. + + The string is now handled properly and the segfault no longer occurs. + + (DER - 2021/04/27, HDFFV-11239) + - HSYS_GOTO_ERROR now emits the results of GetLastError() on Windows HSYS_GOTO_ERROR is an internal macro that is used to produce error diff --git a/src/H5Pfapl.c b/src/H5Pfapl.c index c085b71..fb7b195 100644 --- a/src/H5Pfapl.c +++ b/src/H5Pfapl.c @@ -4221,7 +4221,7 @@ herr_t H5Pset_mdc_log_options(hid_t plist_id, hbool_t is_enabled, const char *location, hbool_t start_on_access) { H5P_genplist_t *plist; /* Property list pointer */ - char * tmp_location; /* Working location pointer */ + char * new_location; /* Working location pointer */ herr_t ret_value = SUCCEED; /* Return value */ FUNC_ENTER_API(FAIL) @@ -4237,19 +4237,14 @@ H5Pset_mdc_log_options(hid_t plist_id, hbool_t is_enabled, const char *location, if (NULL == (plist = H5P_object_verify(plist_id, H5P_FILE_ACCESS))) HGOTO_ERROR(H5E_ARGS, H5E_BADVALUE, FAIL, "plist_id is not a file access property list") - /* Get the current location string and free it */ - if (H5P_get(plist, H5F_ACS_MDC_LOG_LOCATION_NAME, &tmp_location) < 0) - HGOTO_ERROR(H5E_PLIST, H5E_CANTGET, FAIL, "can't get current log location") - H5MM_xfree(tmp_location); - /* Make a copy of the passed-in location */ - if (NULL == (tmp_location = H5MM_xstrdup(location))) + if (NULL == (new_location = H5MM_xstrdup(location))) HGOTO_ERROR(H5E_PLIST, H5E_CANTCOPY, FAIL, "can't copy passed-in log location") /* Set values */ if (H5P_set(plist, H5F_ACS_USE_MDC_LOGGING_NAME, &is_enabled) < 0) HGOTO_ERROR(H5E_PLIST, H5E_CANTSET, FAIL, "can't set is_enabled flag") - if (H5P_set(plist, H5F_ACS_MDC_LOG_LOCATION_NAME, &tmp_location) < 0) + if (H5P_set(plist, H5F_ACS_MDC_LOG_LOCATION_NAME, &new_location) < 0) HGOTO_ERROR(H5E_PLIST, H5E_CANTSET, FAIL, "can't set log location") if (H5P_set(plist, H5F_ACS_START_MDC_LOG_ON_ACCESS_NAME, &start_on_access) < 0) HGOTO_ERROR(H5E_PLIST, H5E_CANTSET, FAIL, "can't set start_on_access flag") diff --git a/test/cache_logging.c b/test/cache_logging.c index 93d4505..12f3d96 100644 --- a/test/cache_logging.c +++ b/test/cache_logging.c @@ -59,6 +59,12 @@ test_logging_api(void) if (H5Pset_mdc_log_options(fapl, is_enabled, LOG_LOCATION, start_on_access) < 0) TEST_ERROR; + /* Ensure that setting the property twice doesn't cause problems + * (addresses a previous bug). + */ + if (H5Pset_mdc_log_options(fapl, is_enabled, LOG_LOCATION, start_on_access) < 0) + TEST_ERROR; + /* Check to make sure that the property list getter returns the correct * location string buffer size; */ -- cgit v0.12