diff options
| -rw-r--r-- | release_docs/RELEASE.txt | 19 | ||||
| -rw-r--r-- | src/H5Fint.c | 4 | ||||
| -rw-r--r-- | src/H5VLnative_file.c | 52 | ||||
| -rw-r--r-- | test/CMakeTests.cmake | 1 | ||||
| -rwxr-xr-x | test/cve_2020_10812.h5 | bin | 0 -> 2565 bytes | |||
| -rw-r--r-- | test/tmisc.c | 39 | ||||
| -rw-r--r-- | tools/src/misc/h5debug.c | 8 |
7 files changed, 97 insertions, 26 deletions
diff --git a/release_docs/RELEASE.txt b/release_docs/RELEASE.txt index 3226165..acbde91 100644 --- a/release_docs/RELEASE.txt +++ b/release_docs/RELEASE.txt @@ -180,6 +180,25 @@ Bug Fixes since HDF5-1.12.1 release =================================== Library ------- + - Seg fault on file close + + h5debug fails at file close with core dump on a file that has an + illegal file size in its cache image. In H5F_dest(), the library + performs all the closing operations for the file and keeps track of + the error encountered when reading the file cache image. + At the end of the routine, it frees the file's file structure and + returns error. Due to the error return, the file object is not removed + from the ID node table. This eventually causes assertion failure in + H5VL__native_file_close() when the library finally exits and tries to + access that file object in the table for closing. + + The closing routine, H5F_dest(), will not free the file structure if + there is error, keeping a valid file structure in the ID node table. + It will be freed later in H5VL__native_file_close() when the + library exits and terminates the file package. + + (VC - 2022/12/14, HDFFV-11052, CVE-2020-10812) + - Fixed an issue with variable length attributes Previously, if a variable length attribute was held open while its file diff --git a/src/H5Fint.c b/src/H5Fint.c index dc86390..1bdf09b 100644 --- a/src/H5Fint.c +++ b/src/H5Fint.c @@ -1648,7 +1648,9 @@ H5F__dest(H5F_t *f, hbool_t flush) if (H5FO_top_dest(f) < 0) HDONE_ERROR(H5E_FILE, H5E_CANTINIT, FAIL, "problems closing file") f->shared = NULL; - f = H5FL_FREE(H5F_t, f); + + if (ret_value >= 0) + f = H5FL_FREE(H5F_t, f); FUNC_LEAVE_NOAPI(ret_value) } /* end H5F__dest() */ diff --git a/src/H5VLnative_file.c b/src/H5VLnative_file.c index 3803e6b..a1a5c7d 100644 --- a/src/H5VLnative_file.c +++ b/src/H5VLnative_file.c @@ -813,29 +813,35 @@ H5VL__native_file_close(void *file, hid_t H5_ATTR_UNUSED dxpl_id, void H5_ATTR_U FUNC_ENTER_PACKAGE /* This routine should only be called when a file ID's ref count drops to zero */ - HDassert(H5F_ID_EXISTS(f)); - - /* Flush file if this is the last reference to this id and we have write - * intent, unless it will be flushed by the "shared" file being closed. - * This is only necessary to replicate previous behaviour, and could be - * disabled by an option/property to improve performance. - */ - if ((H5F_NREFS(f) > 1) && (H5F_INTENT(f) & H5F_ACC_RDWR)) { - /* Get the file ID corresponding to the H5F_t struct */ - if (H5I_find_id(f, H5I_FILE, &file_id) < 0 || H5I_INVALID_HID == file_id) - HGOTO_ERROR(H5E_ATOM, H5E_CANTGET, FAIL, "invalid atom") - - /* Get the number of references outstanding for this file ID */ - if ((nref = H5I_get_ref(file_id, FALSE)) < 0) - HGOTO_ERROR(H5E_ATOM, H5E_CANTGET, FAIL, "can't get ID ref count") - if (nref == 1) - if (H5F__flush(f) < 0) - HGOTO_ERROR(H5E_CACHE, H5E_CANTFLUSH, FAIL, "unable to flush cache") - } /* end if */ - - /* Close the file */ - if (H5F__close(f) < 0) - HGOTO_ERROR(H5E_FILE, H5E_CANTDEC, FAIL, "can't close file") + HDassert(f->shared == NULL || H5F_ID_EXISTS(f)); + + if (f->shared == NULL) + f = H5FL_FREE(H5F_t, f); + + else { + + /* Flush file if this is the last reference to this id and we have write + * intent, unless it will be flushed by the "shared" file being closed. + * This is only necessary to replicate previous behaviour, and could be + * disabled by an option/property to improve performance. + */ + if ((H5F_NREFS(f) > 1) && (H5F_INTENT(f) & H5F_ACC_RDWR)) { + /* Get the file ID corresponding to the H5F_t struct */ + if (H5I_find_id(f, H5I_FILE, &file_id) < 0 || H5I_INVALID_HID == file_id) + HGOTO_ERROR(H5E_ATOM, H5E_CANTGET, FAIL, "invalid atom") + + /* Get the number of references outstanding for this file ID */ + if ((nref = H5I_get_ref(file_id, FALSE)) < 0) + HGOTO_ERROR(H5E_ATOM, H5E_CANTGET, FAIL, "can't get ID ref count") + if (nref == 1) + if (H5F__flush(f) < 0) + HGOTO_ERROR(H5E_CACHE, H5E_CANTFLUSH, FAIL, "unable to flush cache") + } /* end if */ + + /* Close the file */ + if (H5F__close(f) < 0) + HGOTO_ERROR(H5E_FILE, H5E_CANTDEC, FAIL, "can't close file") + } done: FUNC_LEAVE_NOAPI(ret_value) diff --git a/test/CMakeTests.cmake b/test/CMakeTests.cmake index 3dafb2e..6616042 100644 --- a/test/CMakeTests.cmake +++ b/test/CMakeTests.cmake @@ -126,6 +126,7 @@ set (HDF5_REFERENCE_TEST_FILES btree_idx_1_6.h5 btree_idx_1_8.h5 corrupt_stab_msg.h5 + cve_2020_10812.h5 deflate.h5 family_v16_00000.h5 family_v16_00001.h5 diff --git a/test/cve_2020_10812.h5 b/test/cve_2020_10812.h5 Binary files differnew file mode 100755 index 0000000..a20369d --- /dev/null +++ b/test/cve_2020_10812.h5 diff --git a/test/tmisc.c b/test/tmisc.c index ac69f23..3759ea5 100644 --- a/test/tmisc.c +++ b/test/tmisc.c @@ -330,6 +330,11 @@ typedef struct { #define MISC35_SPACE_DIM3 13 #define MISC35_NPOINTS 10 +/* Definitions for misc. test #36 */ +/* The test file is formerly named h5_nrefs_POC. + * See https://nvd.nist.gov/vuln/detail/CVE-2020-10812 */ +#define CVE_2020_10812_FILENAME "cve_2020_10812.h5" + /**************************************************************** ** ** test_misc1(): test unlinking a dataset from a group and immediately @@ -5906,6 +5911,39 @@ test_misc35(void) } /* end test_misc35() */ /**************************************************************** + * ** + * ** test_misc36(): + * ** Test for seg fault issue when closing the provided test file + * ** which has an illegal file size in its cache image. + * ** See HDFFV-11052/CVE-2020-10812 for details. + * ** + * ****************************************************************/ +static void +test_misc36(void) +{ + const char *fname; + hid_t fid; + herr_t ret; + + /* Output message about test being performed */ + MESSAGE(5, ("Fix for HDFFV-11052/CVE-2020-10812")); + + fname = H5_get_srcdir_filename(CVE_2020_10812_FILENAME); + fid = H5Fopen(fname, H5F_ACC_RDONLY, H5P_DEFAULT); + CHECK(fid, FAIL, "H5Fopen"); + + /* This should fail due to the illegal file size. + * It should fail gracefully and not seg fault */ + H5E_BEGIN_TRY + { + ret = H5Fclose(fid); + } + H5E_END_TRY; + VERIFY(ret, FAIL, "H5Fclose"); + +} /* end test_misc36() */ + +/**************************************************************** ** ** test_misc(): Main misc. test routine. ** @@ -5956,6 +5994,7 @@ test_misc(void) test_misc33(); /* Test to verify that H5HL_offset_into() returns error if offset exceeds heap block */ test_misc34(); /* Test behavior of 0 and NULL in H5MM API calls */ test_misc35(); /* Test behavior of free-list & allocation statistics API calls */ + test_misc36(); /* Test for seg fault failure at file close */ } /* test_misc() */ diff --git a/tools/src/misc/h5debug.c b/tools/src/misc/h5debug.c index 31ee131..372ccb5 100644 --- a/tools/src/misc/h5debug.c +++ b/tools/src/misc/h5debug.c @@ -816,8 +816,12 @@ main(int argc, char *argv[]) done: if (fapl > 0) H5Pclose(fapl); - if (fid > 0) - H5Fclose(fid); + if (fid > 0) { + if (H5Fclose(fid) < 0) { + HDfprintf(stderr, "Error in closing file!\n"); + exit_value = 1; + } + } /* Pop API context */ if (api_ctx_pushed) |