philipp/hdf5
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Merge pull request #2706 in HDFFV/hdf5 from ~VCHOI/my_third_fork:bugfix/112_HDFFV-11080-heap-use-after-free-by-the-call to hdf5_1_12

Browse Source 
* commit '02d8c7256e452ff9dc309cbaa8b7bb284c8d6f1e':
  Merge pull request #2693 in HDFFV/hdf5 from ~VCHOI/my_third_fork:bugfix/HDFFV-11080-heap-use-after-free-by-the-call to develop
This commit is contained in:
Vailin Choi
2020-07-23 17:22:01 -05:00
parent ba4933f6d8 02d8c7256e
commit 0a64f3e810
8 changed files with 200 additions and 1 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
MANIFEST
View File

@@ -1240,6 +1240,7 @@
./test/ttsafe.c
./test/ttsafe.h
./test/ttsafe_acreate.c
./test/ttsafe_attr_vlen.c
./test/ttsafe_cancel.c
./test/ttsafe_dcreate.c
./test/ttsafe_error.c

14
release_docs/RELEASE.txt
View File

@@ -134,6 +134,20 @@ Bug Fixes since HDF5-1.12.0 release
==================================
Library
-------
- Fixed the segmentation fault when reading attributes with multiple threads
It was reported that the reading of attributes with variable length string
datatype will crash with segmentation fault particularly when the number of
threads is high (>16 threads). The problem was due to the file pointer that
was set in the variable length string datatype for the attribute. That file
pointer was already closed when the attribute was accessed.
The problem was fixed by setting the file pointer to the current opened file pointer
when the attribute was accessed. Similar patch up was done before when reading
dataset with variable length string datatype.
(VC - 2020/07/13, HDFFV-11080)
- Reduce overhead for H5open(), which is involved in public symbols like
H5T_NATIVE_INT, etc.

4
src/H5Aint.c
View File

@@ -615,6 +615,10 @@ H5A__read(const H5A_t *attr, const H5T_t *mem_type, void *buf)
HDassert(mem_type);
HDassert(buf);
/* Patch the top level file pointer in attr->shared->dt->shared->u.vlen.f if needed */
if(H5T_patch_vlen_file(attr->shared->dt, H5F_VOL_OBJ(attr->oloc.file)) < 0 )
HGOTO_ERROR(H5E_DATASET, H5E_CANTOPENOBJ, FAIL, "can't patch VL datatype file pointer")
/* Create buffer for data to store on disk */
if((snelmts = H5S_GET_EXTENT_NPOINTS(attr->shared->ds)) < 0)
HGOTO_ERROR(H5E_ATTR, H5E_CANTCOUNT, FAIL, "dataspace is invalid")

1
test/CMakeLists.txt
View File

@@ -217,6 +217,7 @@ set (ttsafe_SOURCES
${HDF5_TEST_SOURCE_DIR}/ttsafe_error.c
${HDF5_TEST_SOURCE_DIR}/ttsafe_cancel.c
${HDF5_TEST_SOURCE_DIR}/ttsafe_acreate.c
${HDF5_TEST_SOURCE_DIR}/ttsafe_attr_vlen.c
)
set (H5_TESTS

2
test/Makefile.am
View File

@@ -142,7 +142,7 @@ LDADD=libh5test.la $(LIBHDF5)
# List the source files for tests that have more than one
ttsafe_SOURCES=ttsafe.c ttsafe_dcreate.c ttsafe_error.c ttsafe_cancel.c \
ttsafe_acreate.c
ttsafe_acreate.c ttsafe_attr_vlen.c
cache_image_SOURCES=cache_image.c genall5.c
VFD_LIST = sec2 stdio core core_paged split multi family

1
test/ttsafe.c
View File

@@ -111,6 +111,7 @@ int main(int argc, char *argv[])
AddTest("cancel", tts_cancel, cleanup_cancel, "thread cancellation safety test", NULL);
#endif /* H5_HAVE_PTHREAD_H */
AddTest("acreate", tts_acreate, cleanup_acreate, "multi-attribute creation", NULL);
AddTest("attr_vlen", tts_attr_vlen, cleanup_attr_vlen, "multi-file-attribute-vlen read", NULL);
#else /* H5_HAVE_THREADSAFE */

2
test/ttsafe.h
View File

@@ -35,12 +35,14 @@ void tts_dcreate(void);
void tts_error(void);
void tts_cancel(void);
void tts_acreate(void);
void tts_attr_vlen(void);
/* Prototypes for the cleanup routines */
void cleanup_dcreate(void);
void cleanup_error(void);
void cleanup_cancel(void);
void cleanup_acreate(void);
void cleanup_attr_vlen(void);
#endif /* H5_HAVE_THREADSAFE */
#endif /* TTSAFE_H */

176
test/ttsafe_attr_vlen.c Normal file
View File

@@ -0,0 +1,176 @@
/* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * *
* Copyright by The HDF Group. *
* Copyright by the Board of Trustees of the University of Illinois. *
* All rights reserved. *
* *
* This file is part of HDF5. The full HDF5 copyright notice, including *
* terms governing use, modification, and redistribution, is contained in *
* the COPYING file, which can be found at the root of the source code *
* distribution tree, or in https://support.hdfgroup.org/ftp/HDF5/releases. *
* If you do not have access to either file, you may request a copy from *
* help@hdfgroup.org. *
* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * */
/********************************************************************
*
* Testing for thread safety in H5A library operations.
* ------------------------------------------------------------------
*
* Purpose: Verify that the segmentation fault described in HDFFV-11080
* is fixed.
*
* This test simulates what the user did to trigger the error:
* --Create an HDF5 file
* --Create an attribute with variable length string datatype
* --Attach the attribute to a group
* --Write data to the attribute
* --Close the file
* --Create NUM_THREADS threads
* --For each thread:
* --Open the test file
* --Open and read the attribute for each opened file
*
* The cause of the problem in this jira issue is due to the file pointer
* that is set in the variable length string datatype for the attribute.
* That file pointer is already closed and therefore needs to be set to
* the current opened file pointer when the attribute is accessed.
* Similar patch up was done before when reading dataset in H5D__read()
* in src/H5Aint.c.
* Hopefully this kind of patch can go away when we resolve the
* shared file pointer issue.
*
********************************************************************/
#include "ttsafe.h"
#ifdef H5_HAVE_THREADSAFE
#define FILENAME "ttsafe_attr_vlen.h5"
#define ATTR_NAME "root_attr"
#define NUM_THREADS 32
void *tts_attr_vlen_thread(void *);
void
tts_attr_vlen(void)
{
pthread_t threads[NUM_THREADS] = {0}; /* Thread declaration */
hid_t fid = H5I_INVALID_HID; /* File ID */
hid_t gid = H5I_INVALID_HID; /* Group ID */
hid_t atid = H5I_INVALID_HID; /* Datatype ID for attribute */
hid_t asid = H5I_INVALID_HID; /* Dataspace ID for attribute */
hid_t aid = H5I_INVALID_HID; /* The attribute ID */
char *string_attr = "2.0"; /* The attribute data */
int ret; /* Return value */
int i; /* Local index variable */
/* Create the HDF5 test file */
fid = H5Fcreate(FILENAME, H5F_ACC_TRUNC, H5P_DEFAULT, H5P_DEFAULT);
CHECK(fid, H5I_INVALID_HID, "H5Fcreate");
/* Create variable length string type for attribute */
atid = H5Tcopy(H5T_C_S1);
CHECK(atid, H5I_INVALID_HID, "H5Tcopy");
H5Tset_size(atid, H5T_VARIABLE);
/* Create dataspace for attribute */
asid = H5Screate(H5S_SCALAR);
CHECK(asid, H5I_INVALID_HID, "H5Screate");
/* Open the root group */
gid = H5Gopen2(fid, "/", H5P_DEFAULT);
CHECK(gid, H5I_INVALID_HID, "H5Gopen2");
/* Attach the attribute to the root group */
aid = H5Acreate2(gid, ATTR_NAME, atid, asid, H5P_DEFAULT, H5P_DEFAULT);
CHECK(aid, H5I_INVALID_HID, "H5Acreate2");
/* Write data to the attribute */
ret = H5Awrite(aid, atid, &string_attr);
CHECK(ret, H5I_INVALID_HID, "H5Awrite");
/* Close IDs */
ret = H5Sclose(asid);
CHECK(ret, H5I_INVALID_HID, "H5Sclose");
ret = H5Aclose(aid);
CHECK(ret, H5I_INVALID_HID, "H5Aclose");
ret = H5Gclose(gid);
CHECK(ret, H5I_INVALID_HID, "H5Gclose");
ret = H5Fclose(fid);
CHECK(ret, H5I_INVALID_HID, "H5Fclose");
ret = H5Tclose(atid);
CHECK(ret, H5I_INVALID_HID, "H5Tclose");
/* Start multiple threads and execute tts_attr_vlen_thread() for each thread */
for(i = 0; i < NUM_THREADS; i++)
pthread_create(&threads[i], NULL, tts_attr_vlen_thread, NULL);
/* Wait for the threads to end */
for(i = 0; i < NUM_THREADS; i++)
pthread_join(threads[i], NULL);
} /* end tts_attr_vlen() */
/* Start execution for each thread */
void *
tts_attr_vlen_thread(void *client_data)
{
hid_t fid = H5I_INVALID_HID; /* File ID */
hid_t gid = H5I_INVALID_HID; /* Group ID */
hid_t aid = H5I_INVALID_HID; /* Attribute ID */
hid_t atid = H5I_INVALID_HID; /* Datatype ID for the attribute */
char *string_attr_check; /* The attribute data being read */
char *string_attr = "2.0"; /* The expected attribute data */
herr_t ret; /* Return value */
/* Open the test file */
fid = H5Fopen(FILENAME, H5F_ACC_RDONLY, H5P_DEFAULT);
CHECK(fid, H5I_INVALID_HID, "H5Fopen");
/* Open the group */
gid = H5Gopen2(fid, "/", H5P_DEFAULT);
CHECK(gid, H5I_INVALID_HID, "H5Gopen");
/* Open the attribte */
aid = H5Aopen(gid, "root_attr", H5P_DEFAULT);
CHECK(aid, H5I_INVALID_HID, "H5Aopen");
/* Get the attribute datatype */
atid = H5Aget_type(aid);
CHECK(atid, H5I_INVALID_HID, "H5Aget_type");
/* Read the attribute */
ret = H5Aread(aid, atid, &string_attr_check);
CHECK(ret, FAIL, "H5Aclose");
/* Verify the attribute data is as expected */
VERIFY_STR(string_attr_check, string_attr, "H5Aread");
/* Close IDs */
ret = H5Aclose(aid);
CHECK(ret, FAIL, "H5Aclose");
ret = H5Gclose(gid);
CHECK(ret, FAIL, "H5Aclose");
ret = H5Fclose(fid);
CHECK(ret, FAIL, "H5Aclose");
ret = H5Tclose(atid);
CHECK(ret, FAIL, "H5Aclose");
return NULL;
} /* end tts_attr_vlen_thread() */
void
cleanup_attr_vlen(void)
{
HDunlink(FILENAME);
}
#endif /*H5_HAVE_THREADSAFE*/