From ba9deaeb66ac820f079702fc298c3aea91793a0f Mon Sep 17 00:00:00 2001 From: github-actions <41898282+github-actions[bot]@users.noreply.github.com> Date: Fri, 19 Mar 2021 17:04:16 +0000 Subject: [PATCH 1/3] Committing clang-format changes --- src/H5Tconv.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/H5Tconv.c b/src/H5Tconv.c index f2dff0befbe..0c986331c2c 100644 --- a/src/H5Tconv.c +++ b/src/H5Tconv.c @@ -3843,7 +3843,7 @@ H5T__conv_i_i(hid_t src_id, hid_t dst_id, H5T_cdata_t *cdata, size_t nelmts, siz size_t half_size; /*half the type size */ size_t olap; /*num overlapping elements */ uint8_t * s, *sp, *d, *dp; /*source and dest traversal ptrs*/ - uint8_t * src_rev = NULL; /*order-reversed source buffer */ + uint8_t * src_rev = NULL; /*order-reversed source buffer */ uint8_t dbuf[64] = {0}; /*temp destination buffer */ size_t first; ssize_t sfirst; /*a signed version of `first' */ @@ -4286,7 +4286,7 @@ H5T__conv_f_f(hid_t src_id, hid_t dst_id, H5T_cdata_t *cdata, size_t nelmts, siz size_t olap; /*num overlapping elements */ ssize_t bitno = 0; /*bit number */ uint8_t * s, *sp, *d, *dp; /*source and dest traversal ptrs*/ - uint8_t * src_rev = NULL; /*order-reversed source buffer */ + uint8_t * src_rev = NULL; /*order-reversed source buffer */ uint8_t dbuf[64] = {0}; /*temp destination buffer */ uint8_t tmp1, tmp2; /*temp variables for swapping bytes*/ @@ -8401,7 +8401,7 @@ H5T__conv_f_i(hid_t src_id, hid_t dst_id, H5T_cdata_t *cdata, size_t nelmts, siz size_t tsize; /*type size for swapping bytes */ size_t olap; /*num overlapping elements */ uint8_t * s, *sp, *d, *dp; /*source and dest traversal ptrs*/ - uint8_t * src_rev = NULL; /*order-reversed source buffer */ + uint8_t * src_rev = NULL; /*order-reversed source buffer */ uint8_t dbuf[64] = {0}; /*temp destination buffer */ uint8_t tmp1, tmp2; /*temp variables for swapping bytes*/ @@ -9027,7 +9027,7 @@ H5T__conv_i_f(hid_t src_id, hid_t dst_id, H5T_cdata_t *cdata, size_t nelmts, siz size_t tsize; /*type size for swapping bytes */ size_t olap; /*num overlapping elements */ uint8_t * s, *sp, *d, *dp; /*source and dest traversal ptrs*/ - uint8_t * src_rev = NULL; /*order-reversed source buffer */ + uint8_t * src_rev = NULL; /*order-reversed source buffer */ uint8_t dbuf[64] = {0}; /*temp destination buffer */ uint8_t tmp1, tmp2; /*temp variables for swapping bytes*/ From 568fea5594a4e21e00c3732b0735c5b0a1aaa55b Mon Sep 17 00:00:00 2001 From: Dana Robinson Date: Tue, 27 Apr 2021 18:36:33 -0700 Subject: [PATCH 2/3] 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 --- 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 359f222148d..9632f0ba7eb 100644 --- a/release_docs/RELEASE.txt +++ b/release_docs/RELEASE.txt @@ -414,6 +414,18 @@ New Features Library: -------- + - 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 property and the sefault 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 c085b718860..fb7b195a8dd 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 93d450511d1..12f3d9631b0 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; */ From 3b5d2b8622d08747a6510dd39f24130e50aa50a3 Mon Sep 17 00:00:00 2001 From: Dana Robinson Date: Wed, 28 Apr 2021 08:38:58 -0700 Subject: [PATCH 3/3] Fixes typos --- release_docs/RELEASE.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/release_docs/RELEASE.txt b/release_docs/RELEASE.txt index 9632f0ba7eb..695eacefd3a 100644 --- a/release_docs/RELEASE.txt +++ b/release_docs/RELEASE.txt @@ -422,7 +422,7 @@ New Features 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 property and the sefault no longer occurs. + The string is now handled properly and the segfault no longer occurs. (DER - 2021/04/27, HDFFV-11239)