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

* commit '3023b34272cc6ada84aebfa7441a6a55afa3621c':
  Update MANIFEST. Add new test to Cmake.
  Fix for jira issue HDFFV-11080: (1) Patch up the file pointer when reading attribute of variable length datatype (2) Test to verify the fix when doing multiple threads

7 files changed, 198 insertions, 1 deletions

diff --git a/MANIFEST b/MANIFEST
index b998f69..5d5bf59 100644
--- a/MANIFEST
+++ b/MANIFEST
@@ -1191,6 +1191,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
diff --git a/release_docs/RELEASE.txt b/release_docs/RELEASE.txt
index f76f2e5..9d1eab1 100644
--- a/release_docs/RELEASE.txt
+++ b/release_docs/RELEASE.txt
@@ -342,6 +342,20 @@ Bug Fixes since HDF5-1.10.5 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)
+
     - Fixed CVE-2018-17435
 
       The tool h52gif produced a segfault when the size of an attribute
diff --git a/src/H5Aint.c b/src/H5Aint.c
index f923e24..d388fb5 100644
--- a/src/H5Aint.c
+++ b/src/H5Aint.c
@@ -613,6 +613,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")
diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt
index 4cb94a2..8e553d0 100644
--- a/test/CMakeLists.txt
+++ b/test/CMakeLists.txt
@@ -179,6 +179,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
diff --git a/test/Makefile.am b/test/Makefile.am
index 3996f7e..f60f55c 100644
--- a/test/Makefile.am
+++ b/test/Makefile.am
@@ -135,7 +135,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
diff --git a/test/ttsafe.c b/test/ttsafe.c
index e6edd9a..2de460b 100644
--- a/test/ttsafe.c
+++ b/test/ttsafe.c
@@ -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 */
 
diff --git a/test/ttsafe_attr_vlen.c b/test/ttsafe_attr_vlen.c
new file mode 100644
index 0000000..b07e362
--- /dev/null
+++ b/test/ttsafe_attr_vlen.c
@@ -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*/
+