From 4e31361dad4add06792b652dbe5b97e501f9031d Mon Sep 17 00:00:00 2001 From: Songyu Lu Date: Tue, 12 Feb 2019 13:48:49 -0600 Subject: 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 | 25 ++++++------- src/H5HG.c | 8 ++++- src/H5Ocache.c | 2 +- src/H5Olayout.c | 8 ++++- src/H5RS.c | 4 +-- src/H5RSprivate.h | 2 +- test/tvlstr.c | 102 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 7 files changed, 133 insertions(+), 18 deletions(-) diff --git a/src/H5Dchunk.c b/src/H5Dchunk.c index 3fa4a23..0be19bd 100644 --- a/src/H5Dchunk.c +++ b/src/H5Dchunk.c @@ -713,6 +713,9 @@ H5D__chunk_set_info_real(H5O_layout_chunk_t *layout, unsigned ndims, /* Compute the # of chunks in dataset dimensions */ for(u = 0, layout->nchunks = 1, layout->max_nchunks = 1; u < ndims; u++) { + /* Sanity check */ + HDassert(layout->dim[u] > 0); + /* Round up to the next integer # of chunks, to accommodate partial chunks */ layout->chunks[u] = ((curr_dims[u] + layout->dim[u]) - 1) / layout->dim[u]; if(H5S_UNLIMITED == max_dims[u]) @@ -967,7 +970,10 @@ H5D__chunk_init(H5F_t *f, const H5D_t *dset, hid_t dapl_id) /* Initial scaled dimension sizes */ if(dset->shared->layout.u.chunk.dim[u] == 0) HGOTO_ERROR(H5E_DATASET, H5E_BADVALUE, FAIL, "chunk size must be > 0, dim = %u ", u) - rdcc->scaled_dims[u] = dset->shared->curr_dims[u] / dset->shared->layout.u.chunk.dim[u]; + + /* Round up to the next integer # of chunks, to accommodate partial chunks */ + rdcc->scaled_dims[u] = (dset->shared->curr_dims[u] + dset->shared->layout.u.chunk.dim[u] - 1) / + dset->shared->layout.u.chunk.dim[u]; if( !(scaled_power2up = H5VM_power2up(rdcc->scaled_dims[u])) ) HGOTO_ERROR(H5E_DATASET, H5E_CANTGET, FAIL, "unable to get the next power of 2") @@ -2821,6 +2827,7 @@ H5D__chunk_hash_val(const H5D_shared_t *shared, const hsize_t *scaled) hsize_t val; /* Intermediate value */ unsigned ndims = shared->ndims; /* Rank of dataset */ unsigned ret = 0; /* Value to return */ + unsigned u; /* Local index variable */ FUNC_ENTER_STATIC_NOERR @@ -2831,17 +2838,11 @@ H5D__chunk_hash_val(const H5D_shared_t *shared, const hsize_t *scaled) /* If the fastest changing dimension doesn't have enough entropy, use * other dimensions too */ - if(ndims > 1 && shared->cache.chunk.scaled_dims[ndims - 1] <= shared->cache.chunk.nslots) { - unsigned u; /* Local index variable */ - - val = scaled[0]; - for(u = 1; u < ndims; u++) { - val <<= shared->cache.chunk.scaled_encode_bits[u]; - val ^= scaled[u]; - } /* end for */ - } /* end if */ - else - val = scaled[ndims - 1]; + val = scaled[0]; + for(u = 1; u < ndims; u++) { + val <<= shared->cache.chunk.scaled_encode_bits[u]; + val ^= scaled[u]; + } /* end for */ /* Modulo value against the number of array slots */ ret = (unsigned)(val % shared->cache.chunk.nslots); diff --git a/src/H5HG.c b/src/H5HG.c index 53f1b2a..29cd90f 100644 --- a/src/H5HG.c +++ b/src/H5HG.c @@ -785,7 +785,13 @@ H5HG_remove (H5F_t *f, 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/H5Ocache.c b/src/H5Ocache.c index 1fa4b10..d63ceb1 100644 --- a/src/H5Ocache.c +++ b/src/H5Ocache.c @@ -1627,7 +1627,7 @@ H5O__chunk_deserialize(H5O_t *oh, haddr_t addr, size_t len, const uint8_t *image done: if(ret_value < 0 && udata->cont_msg_info->msgs) { - udata->cont_msg_info->msgs = (H5O_chunk_t *)H5FL_SEQ_FREE(H5O_cont_t, udata->cont_msg_info->msgs); + udata->cont_msg_info->msgs = H5FL_SEQ_FREE(H5O_cont_t, udata->cont_msg_info->msgs); udata->cont_msg_info->alloc_nmsgs = 0; } FUNC_LEAVE_NOAPI(ret_value) diff --git a/src/H5Olayout.c b/src/H5Olayout.c index c880435..d0c7f3e 100644 --- a/src/H5Olayout.c +++ b/src/H5Olayout.c @@ -243,9 +243,15 @@ H5O__layout_decode(H5F_t *f, H5O_t H5_ATTR_UNUSED *open_oh, 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 3315d68..58713e7 100644 --- a/test/tvlstr.c +++ b/test/tvlstr.c @@ -23,10 +23,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) } /* 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. ** @@ -869,6 +968,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() */ @@ -891,5 +992,6 @@ cleanup_vlstrings(void) { HDremove(DATAFILE); HDremove(DATAFILE2); + HDremove(DATAFILE3); } -- cgit v0.12