From 4e31361dad4add06792b652dbe5b97e501f9031d Mon Sep 17 00:00:00 2001 From: Songyu Lu Date: Tue, 12 Feb 2019 13:48:49 -0600 Subject: [PATCH] I'm bringing the fixes for the following Jira issues from the develop branch to 1.10 branch: HDFFV-10571: Divided by Zero vulnerability. HDFFV-10601: Issues with chunk cache hash value calcuation. HDFFV-10607: Patches for warnings in the core libraries. HDFFV-10635: HDF5 library segmentation fault with H5Sselect_element. --- src/H5Dchunk.c | 3 ++ src/H5HG.c | 8 +++- src/H5Olayout.c | 8 +++- src/H5RS.c | 4 +- src/H5RSprivate.h | 2 +- test/tvlstr.c | 102 ++++++++++++++++++++++++++++++++++++++++++++++ 6 files changed, 122 insertions(+), 5 deletions(-) diff --git a/src/H5Dchunk.c b/src/H5Dchunk.c index 021335f..6183ad8 100644 --- a/src/H5Dchunk.c +++ b/src/H5Dchunk.c @@ -641,6 +641,9 @@ H5D__chunk_set_info_real(H5O_layout_chunk_t *layout, unsigned ndims, const hsize /* Compute the # of chunks in dataset dimensions */ for(u = 0, layout->nchunks = 1; u < ndims; u++) { + /* Sanity check */ + HDassert(layout->dim[u] > 0); + /* Round up to the next integer # of chunks, to accomodate partial chunks */ layout->chunks[u] = ((curr_dims[u] + layout->dim[u]) - 1) / layout->dim[u]; diff --git a/src/H5HG.c b/src/H5HG.c index 194e4eb..e07ca0e 100644 --- a/src/H5HG.c +++ b/src/H5HG.c @@ -757,7 +757,13 @@ H5HG_remove (H5F_t *f, hid_t dxpl_id, H5HG_t *hobj) HGOTO_ERROR(H5E_HEAP, H5E_CANTPROTECT, FAIL, "unable to protect global heap") HDassert(hobj->idx < heap->nused); - HDassert(heap->obj[hobj->idx].begin); + /* When the application selects the same location to rewrite the VL element by using H5Sselect_elements, + * it can happen that the entry has been removed by first rewrite. Here we simply skip the removal of + * the entry and let the second rewrite happen (see HDFFV-10635). In the future, it'd be nice to handle + * this situation in H5T_conv_vlen in H5Tconv.c instead of this level (HDFFV-10648). */ + if(heap->obj[hobj->idx].nrefs == 0 && heap->obj[hobj->idx].size == 0 && !heap->obj[hobj->idx].begin) + HGOTO_DONE(ret_value) + obj_start = heap->obj[hobj->idx].begin; /* Include object header size */ need = H5HG_ALIGN(heap->obj[hobj->idx].size) + H5HG_SIZEOF_OBJHDR(f); diff --git a/src/H5Olayout.c b/src/H5Olayout.c index 17385c2..3bbee12 100644 --- a/src/H5Olayout.c +++ b/src/H5Olayout.c @@ -223,9 +223,15 @@ H5O_layout_decode(H5F_t *f, hid_t H5_ATTR_UNUSED dxpl_id, H5O_t H5_ATTR_UNUSED * H5F_addr_decode(f, &p, &(mesg->storage.u.chunk.idx_addr)); /* Chunk dimensions */ - for(u = 0; u < mesg->u.chunk.ndims; u++) + for(u = 0; u < mesg->u.chunk.ndims; u++) { UINT32DECODE(p, mesg->u.chunk.dim[u]); + /* Just in case that something goes very wrong, such as file corruption. */ + if(mesg->u.chunk.dim[u] == 0) + HGOTO_ERROR(H5E_DATASET, H5E_BADVALUE, NULL, "chunk dimension must be positive: mesg->u.chunk.dim[%u] = %u", + u, mesg->u.chunk.dim[u]) + } /* end for */ + /* Compute chunk size */ for(u = 1, mesg->u.chunk.size = mesg->u.chunk.dim[0]; u < mesg->u.chunk.ndims; u++) mesg->u.chunk.size *= mesg->u.chunk.dim[u]; diff --git a/src/H5RS.c b/src/H5RS.c index 0a3fff0..ae500c5 100644 --- a/src/H5RS.c +++ b/src/H5RS.c @@ -137,7 +137,7 @@ done: REVISION LOG --------------------------------------------------------------------------*/ H5RS_str_t * -H5RS_wrap(char *s) +H5RS_wrap(const char *s) { H5RS_str_t *ret_value; /* Return value */ @@ -148,7 +148,7 @@ H5RS_wrap(char *s) HGOTO_ERROR(H5E_RS, H5E_NOSPACE, NULL, "memory allocation failed") /* Set the internal fields */ - ret_value->s = s; + ret_value->s = (char *)s; ret_value->wrapped = 1; ret_value->n = 1; diff --git a/src/H5RSprivate.h b/src/H5RSprivate.h index f69624a..1d26a18 100644 --- a/src/H5RSprivate.h +++ b/src/H5RSprivate.h @@ -44,7 +44,7 @@ typedef struct H5RS_str_t H5RS_str_t; /* Private routines */ /********************/ H5_DLL H5RS_str_t *H5RS_create(const char *s); -H5_DLL H5RS_str_t *H5RS_wrap(char *s); +H5_DLL H5RS_str_t *H5RS_wrap(const char *s); H5_DLL H5RS_str_t *H5RS_own(char *s); H5_DLL herr_t H5RS_decr(H5RS_str_t *rs); H5_DLL herr_t H5RS_incr(H5RS_str_t *rs); diff --git a/test/tvlstr.c b/test/tvlstr.c index e5b2a60..bb860a2 100644 --- a/test/tvlstr.c +++ b/test/tvlstr.c @@ -25,10 +25,14 @@ #define DATAFILE "tvlstr.h5" #define DATAFILE2 "tvlstr2.h5" +#define DATAFILE3 "sel2el.h5" + +#define DATASET "1Darray" /* 1-D dataset with fixed dimensions */ #define SPACE1_RANK 1 #define SPACE1_DIM1 4 +#define NUMP 4 #define VLSTR_TYPE "vl_string_type" @@ -846,6 +850,101 @@ static void test_vl_rewrite(void) return; } /* end test_vl_rewrite() */ +/**************************************************************** + ** + ** test_write_same_element(): + ** Tests writing to the same element of VL string using + ** H5Sselect_element. + ** + ****************************************************************/ +static void test_write_same_element(void) +{ + hid_t file1, dataset1; + hid_t mspace, fspace, dtype; + hsize_t fdim[] = {SPACE1_DIM1}; + char *val[SPACE1_DIM1] = {"But", "reuniting", "is a", "great joy"}; + hsize_t marray[] = {NUMP}; + hsize_t coord[SPACE1_RANK][NUMP]; + herr_t ret; + + char *wdata[SPACE1_DIM1] = {"Parting", "is such a", "sweet", "sorrow."}; + + file1 = H5Fcreate(DATAFILE3, H5F_ACC_TRUNC, H5P_DEFAULT, H5P_DEFAULT); + CHECK(file1, FAIL, "H5Fcreate"); + + dtype = H5Tcopy(H5T_C_S1); + CHECK(dtype, FAIL, "H5Tcopy"); + + ret = H5Tset_size(dtype, H5T_VARIABLE); + CHECK(ret, FAIL, "H5Tset_size"); + + fspace = H5Screate_simple(SPACE1_RANK, fdim, NULL); + CHECK(fspace, FAIL, "H5Screate_simple"); + + dataset1 = H5Dcreate(file1, DATASET, dtype, fspace, H5P_DEFAULT, + H5P_DEFAULT, H5P_DEFAULT); + CHECK(dataset1, FAIL, "H5Dcreate"); + + ret = H5Dwrite(dataset1, dtype, H5S_ALL, H5S_ALL, H5P_DEFAULT, wdata); + CHECK(ret, FAIL, "H5Dwrite"); + + ret = H5Dclose(dataset1); + CHECK(ret, FAIL, "H5Dclose"); + + ret = H5Tclose(dtype); + CHECK(ret, FAIL, "H5Tclose"); + + ret = H5Sclose(fspace); + CHECK(ret, FAIL, "H5Sclose"); + + ret = H5Fclose(file1); + CHECK(ret, FAIL, "H5Fclose"); + + /* + * Open the file. Select the same points, write values to those point locations. + */ + file1 = H5Fopen(DATAFILE3, H5F_ACC_RDWR, H5P_DEFAULT); + CHECK(file1, FAIL, "H5Fopen"); + + dataset1 = H5Dopen(file1, DATASET, H5P_DEFAULT); + CHECK(dataset1, FAIL, "H5Dopen"); + + fspace = H5Dget_space(dataset1); + CHECK(fspace, FAIL, "H5Dget_space"); + + dtype = H5Dget_type(dataset1); + CHECK(dtype, FAIL, "H5Dget_type"); + + mspace = H5Screate_simple(1, marray, NULL); + CHECK(mspace, FAIL, "H5Screate_simple"); + + coord[0][0] = 0; + coord[0][1] = 2; + coord[0][2] = 2; + coord[0][3] = 0; + + ret = H5Sselect_elements(fspace, H5S_SELECT_SET, NUMP, (const hsize_t *)&coord); + CHECK(ret, FAIL, "H5Sselect_elements"); + + ret = H5Dwrite(dataset1, dtype, mspace, fspace, H5P_DEFAULT, val); + CHECK(ret, FAIL, "H5Dwrite"); + + ret = H5Tclose(dtype); + CHECK(ret, FAIL, "H5Tclose"); + + ret = H5Dclose(dataset1); + CHECK(ret, FAIL, "H5Dclose"); + + ret = H5Sclose(fspace); + CHECK(ret, FAIL, "H5Dclose"); + + ret = H5Sclose(mspace); + CHECK(ret, FAIL, "H5Sclose"); + + ret = H5Fclose(file1); + CHECK(ret, FAIL, "H5Fclose"); +} /* test_write_same_element */ + /**************************************************************** ** ** test_vlstrings(): Main VL string testing routine. @@ -870,6 +969,8 @@ test_vlstrings(void) /* Test writing VL datasets in files with lots of unlinking */ test_vl_rewrite(); + /* Test writing to the same element more than once using H5Sselect_elements */ + test_write_same_element(); } /* test_vlstrings() */ @@ -892,5 +993,6 @@ cleanup_vlstrings(void) { HDremove(DATAFILE); HDremove(DATAFILE2); + HDremove(DATAFILE3); } -- 2.23.0