From d767e6a067aacee0d33a51d5c584d6676afc3df9 Mon Sep 17 00:00:00 2001 From: Dana Robinson Date: Tue, 18 Jun 2019 11:42:53 -0700 Subject: Fixed a bug in the links code where iterating over an empty group would pass a NULL pointer to qsort(3), which is undefined behavior. Fixes HDFFV-10829 --- release_docs/RELEASE.txt | 12 ++++++++++++ src/H5Glink.c | 16 +++++++++++----- 2 files changed, 23 insertions(+), 5 deletions(-) diff --git a/release_docs/RELEASE.txt b/release_docs/RELEASE.txt index 78118b8..fc236bb 100644 --- a/release_docs/RELEASE.txt +++ b/release_docs/RELEASE.txt @@ -375,6 +375,18 @@ Bug Fixes since HDF5-1.10.3 release (JTH - 2018/08/25, HDFFV-10501) + - When iterating over an old-style group (i.e., when not using the latest + file format) of size 0, a NULL pointer representing the empty links + table would be sent to qsort(3) for sorting, which is undefined behavior. + + Iterating over an empty group is explicitly tested in the links test. + This has not caused any failures to date and was flagged by gcc's + -fsanitize=undefined. + + The library no longer attempts to sort an empty array. + + (DER - 2019/06/18, HDFFV-10829) + Java Library: ---------------- - JNI native library dependencies diff --git a/src/H5Glink.c b/src/H5Glink.c index 82a2dcf..89f0266 100644 --- a/src/H5Glink.c +++ b/src/H5Glink.c @@ -402,15 +402,14 @@ done: /*------------------------------------------------------------------------- - * Function: H5G__link_sort_table + * Function: H5G__link_sort_table * * Purpose: Sort table containing a list of links for a group * - * Return: Success: Non-negative - * Failure: Negative + * Return: SUCCEED/FAIL * - * Programmer: Quincey Koziol - * Nov 20, 2006 + * Programmer: Quincey Koziol + * Nov 20, 2006 * *------------------------------------------------------------------------- */ @@ -423,6 +422,13 @@ H5G__link_sort_table(H5G_link_table_t *ltable, H5_index_t idx_type, /* Sanity check */ HDassert(ltable); + /* Can't sort when empty since the links table will be NULL */ + if(0 == ltable->nlinks) + return SUCCEED; + + /* This should never be NULL if the number of links is non-zero */ + HDassert(ltable->lnks); + /* Pick appropriate sorting routine */ if(idx_type == H5_INDEX_NAME) { if(order == H5_ITER_INC) -- cgit v0.12 From ea966597d513e1df8feb96887cd51aa0e48c85ba Mon Sep 17 00:00:00 2001 From: Dana Robinson Date: Tue, 18 Jun 2019 12:23:06 -0700 Subject: Updated the HDqsort() macro to ensure we don't pass NULL buffers to qsort(3) in the future. --- src/H5private.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/H5private.h b/src/H5private.h index dad2d94..06592eb 100644 --- a/src/H5private.h +++ b/src/H5private.h @@ -1171,7 +1171,7 @@ typedef off_t h5_stat_size_t; #define HDpwrite(F,B,C,O) pwrite(F,B,C,O) #endif /* HDpwrite */ #ifndef HDqsort - #define HDqsort(M,N,Z,F) qsort(M,N,Z,F) + #define HDqsort(M,N,Z,F) do {HDassert(M); qsort(M,N,Z,F);} while(0) #endif /* HDqsort*/ #ifndef HDraise #define HDraise(N) raise(N) -- cgit v0.12 From b8c24388ece6f6c0fb25eac5ca4b7c0bda8d89c5 Mon Sep 17 00:00:00 2001 From: Dana Robinson Date: Tue, 18 Jun 2019 12:27:51 -0700 Subject: Switched to HGOTO_DONE() in the links code. --- src/H5Glink.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/H5Glink.c b/src/H5Glink.c index 89f0266..04ccbc5 100644 --- a/src/H5Glink.c +++ b/src/H5Glink.c @@ -417,6 +417,8 @@ herr_t H5G__link_sort_table(H5G_link_table_t *ltable, H5_index_t idx_type, H5_iter_order_t order) { + herr_t ret_value = SUCCEED; + FUNC_ENTER_PACKAGE_NOERR /* Sanity check */ @@ -424,7 +426,7 @@ H5G__link_sort_table(H5G_link_table_t *ltable, H5_index_t idx_type, /* Can't sort when empty since the links table will be NULL */ if(0 == ltable->nlinks) - return SUCCEED; + HGOTO_DONE(ret_value); /* This should never be NULL if the number of links is non-zero */ HDassert(ltable->lnks); @@ -448,6 +450,7 @@ H5G__link_sort_table(H5G_link_table_t *ltable, H5_index_t idx_type, HDassert(order == H5_ITER_NATIVE); } /* end else */ +done: FUNC_LEAVE_NOAPI(SUCCEED) } /* end H5G__link_sort_table() */ -- cgit v0.12 From 0a75da70a61d41a0bf20a5c617f3eb678ed0b103 Mon Sep 17 00:00:00 2001 From: Dana Robinson Date: Tue, 18 Jun 2019 12:52:42 -0700 Subject: Yanked qsort assert --- src/H5private.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/H5private.h b/src/H5private.h index 06592eb..dad2d94 100644 --- a/src/H5private.h +++ b/src/H5private.h @@ -1171,7 +1171,7 @@ typedef off_t h5_stat_size_t; #define HDpwrite(F,B,C,O) pwrite(F,B,C,O) #endif /* HDpwrite */ #ifndef HDqsort - #define HDqsort(M,N,Z,F) do {HDassert(M); qsort(M,N,Z,F);} while(0) + #define HDqsort(M,N,Z,F) qsort(M,N,Z,F) #endif /* HDqsort*/ #ifndef HDraise #define HDraise(N) raise(N) -- cgit v0.12