diff --git a/release_docs/RELEASE.txt b/release_docs/RELEASE.txt index db2a6df4bb7..02e0c731b45 100644 --- a/release_docs/RELEASE.txt +++ b/release_docs/RELEASE.txt @@ -241,6 +241,32 @@ Bug Fixes since HDF5-1.12.2 release =================================== Library ------- + - Fixed CVE-2018-11202 + + A malformed file could result in chunk index memory leaks. Under most + conditions (i.e., when the --enable-using-memchecker option is NOT + used), this would result in a small memory leak and and infinite loop + and abort when shutting down the library. The infinite loop would be + due to the "free list" package not being able to clear its resources + so the library couldn't shut down. When the "using a memory checker" + option is used, the free lists are disabled so there is just a memory + leak with no abort on library shutdown. + + The chunk index resources are now correctly cleaned up when reading + misparsed files and valgrind confirms no memory leaks. + + - Fixed an assertion in a previous fix for CVE-2016-4332 + + An assert could fail when processing corrupt files that have invalid + shared message flags (as in CVE-2016-4332). + + The assert statement in question has been replaced with pointer checks + that don't raise errors. Since the function is in cleanup code, we do + our best to close and free things, even when presented with partially + initialized structs. + + Fixes CVE-2016-4332 and HDFFV-9950 (confirmed via the cve_hdf5 repo) + - Fixed a file space allocation bug in the parallel library for chunked datasets diff --git a/src/H5Dbtree.c b/src/H5Dbtree.c index c3aee44fd6c..12d151f5f86 100644 --- a/src/H5Dbtree.c +++ b/src/H5Dbtree.c @@ -661,7 +661,8 @@ H5D__btree_decode_key(const H5B_shared_t *shared, const uint8_t *raw, void *_key /* Retrieve coordinate offset */ UINT64DECODE(raw, tmp_offset); - HDassert(0 == (tmp_offset % layout->dim[u])); + if (0 != (tmp_offset % layout->dim[u])) + HGOTO_ERROR(H5E_DATASET, H5E_BADVALUE, FAIL, "bad coordinate offset"); /* Convert to a scaled offset */ key->scaled[u] = tmp_offset / layout->dim[u]; diff --git a/src/H5Dchunk.c b/src/H5Dchunk.c index a0f83714627..b5e1d5ea9bc 100644 --- a/src/H5Dchunk.c +++ b/src/H5Dchunk.c @@ -701,9 +701,12 @@ H5D__chunk_set_info_real(H5O_layout_chunk_t *layout, unsigned ndims, const hsize /* Sanity checks */ HDassert(layout); - HDassert(ndims > 0); HDassert(curr_dims); + /* Can happen when corrupt files are parsed */ + if (ndims == 0) + HGOTO_ERROR(H5E_DATASET, H5E_BADVALUE, FAIL, "number of dimensions cannot be zero") + /* Compute the # of chunks in dataset dimensions */ for (u = 0, layout->nchunks = 1, layout->max_nchunks = 1; u < ndims; u++) { /* Round up to the next integer # of chunks, to accommodate partial chunks */ @@ -915,6 +918,7 @@ H5D__chunk_init(H5F_t *f, const H5D_t *const dset, hid_t dapl_id) H5D_rdcc_t *rdcc = &(dset->shared->cache.chunk); /* Convenience pointer to dataset's chunk cache */ H5P_genplist_t *dapl; /* Data access property list object pointer */ H5O_storage_chunk_t *sc = &(dset->shared->layout.storage.u.chunk); + hbool_t idx_init = FALSE; herr_t ret_value = SUCCEED; /* Return value */ FUNC_ENTER_STATIC @@ -990,12 +994,21 @@ H5D__chunk_init(H5F_t *f, const H5D_t *const dset, hid_t dapl_id) /* Allocate any indexing structures */ if (sc->ops->init && (sc->ops->init)(&idx_info, dset->shared->space, dset->oloc.addr) < 0) HGOTO_ERROR(H5E_DATASET, H5E_CANTINIT, FAIL, "can't initialize indexing information") + idx_init = TRUE; /* Set the number of chunks in dataset, etc. */ if (H5D__chunk_set_info(dset) < 0) HGOTO_ERROR(H5E_DATASET, H5E_CANTINIT, FAIL, "unable to set # of chunks for dataset") done: + if (FAIL == ret_value) { + if (rdcc->slot) + rdcc->slot = H5FL_SEQ_FREE(H5D_rdcc_ent_ptr_t, rdcc->slot); + + if (idx_init && sc->ops->dest && (sc->ops->dest)(&idx_info) < 0) + HDONE_ERROR(H5E_DATASET, H5E_CANTFREE, FAIL, "unable to release chunk index info"); + } + FUNC_LEAVE_NOAPI(ret_value) } /* end H5D__chunk_init() */ diff --git a/src/H5Fsuper_cache.c b/src/H5Fsuper_cache.c index df61252cac3..0c297564d6b 100644 --- a/src/H5Fsuper_cache.c +++ b/src/H5Fsuper_cache.c @@ -409,13 +409,13 @@ H5F__cache_superblock_verify_chksum(const void *_image, size_t len, void *_udata *------------------------------------------------------------------------- */ static void * -H5F__cache_superblock_deserialize(const void *_image, size_t H5_ATTR_NDEBUG_UNUSED len, void *_udata, - hbool_t H5_ATTR_UNUSED *dirty) +H5F__cache_superblock_deserialize(const void *_image, size_t len, void *_udata, hbool_t H5_ATTR_UNUSED *dirty) { H5F_super_t *sblock = NULL; /* File's superblock */ H5F_superblock_cache_ud_t *udata = (H5F_superblock_cache_ud_t *)_udata; /* User data */ - const uint8_t *image = _image; /* Pointer into raw data buffer */ - H5F_super_t *ret_value = NULL; /* Return value */ + const uint8_t *image = _image; /* Pointer into raw data buffer */ + const uint8_t *end = image + len - 1; /* Pointer to end of buffer */ + H5F_super_t *ret_value = NULL; FUNC_ENTER_STATIC @@ -427,11 +427,11 @@ H5F__cache_superblock_deserialize(const void *_image, size_t H5_ATTR_NDEBUG_UNUS /* Allocate space for the superblock */ if (NULL == (sblock = H5FL_CALLOC(H5F_super_t))) - HGOTO_ERROR(H5E_RESOURCE, H5E_NOSPACE, NULL, "memory allocation failed") + HGOTO_ERROR(H5E_RESOURCE, H5E_NOSPACE, NULL, "memory allocation failed"); /* Deserialize the file superblock's prefix */ if (H5F__superblock_prefix_decode(sblock, &image, udata, FALSE) < 0) - HGOTO_ERROR(H5E_FILE, H5E_CANTDECODE, NULL, "can't decode file superblock prefix") + HGOTO_ERROR(H5E_FILE, H5E_CANTDECODE, NULL, "can't decode file superblock prefix"); /* Check for older version of superblock format */ if (sblock->super_vers < HDF5_SUPERBLOCK_VERSION_2) { @@ -441,85 +441,113 @@ H5F__cache_superblock_deserialize(const void *_image, size_t H5_ATTR_NDEBUG_UNUS unsigned chunk_btree_k; /* B-tree chunk internal node 'K' value */ /* Freespace version (hard-wired) */ + if (H5_IS_BUFFER_OVERFLOW(image, 1, end)) + HGOTO_ERROR(H5E_FILE, H5E_OVERFLOW, NULL, "image pointer is out of bounds"); if (HDF5_FREESPACE_VERSION != *image++) - HGOTO_ERROR(H5E_FILE, H5E_BADVALUE, NULL, "bad free space version number") + HGOTO_ERROR(H5E_FILE, H5E_BADVALUE, NULL, "bad free space version number"); /* Root group version number (hard-wired) */ + if (H5_IS_BUFFER_OVERFLOW(image, 1, end)) + HGOTO_ERROR(H5E_FILE, H5E_OVERFLOW, NULL, "image pointer is out of bounds"); if (HDF5_OBJECTDIR_VERSION != *image++) - HGOTO_ERROR(H5E_FILE, H5E_BADVALUE, NULL, "bad object directory version number") + HGOTO_ERROR(H5E_FILE, H5E_BADVALUE, NULL, "bad object directory version number"); /* Skip over reserved byte */ + if (H5_IS_BUFFER_OVERFLOW(image, 1, end)) + HGOTO_ERROR(H5E_FILE, H5E_OVERFLOW, NULL, "image pointer is out of bounds"); image++; /* Shared header version number (hard-wired) */ + if (H5_IS_BUFFER_OVERFLOW(image, 1, end)) + HGOTO_ERROR(H5E_FILE, H5E_OVERFLOW, NULL, "image pointer is out of bounds"); if (HDF5_SHAREDHEADER_VERSION != *image++) - HGOTO_ERROR(H5E_FILE, H5E_BADVALUE, NULL, "bad shared-header format version number") + HGOTO_ERROR(H5E_FILE, H5E_BADVALUE, NULL, "bad shared-header format version number"); /* Skip over size of file addresses (already decoded) */ + if (H5_IS_BUFFER_OVERFLOW(image, 1, end)) + HGOTO_ERROR(H5E_FILE, H5E_OVERFLOW, NULL, "image pointer is out of bounds"); image++; udata->f->shared->sizeof_addr = sblock->sizeof_addr; /* Keep a local copy also */ /* Skip over size of file sizes (already decoded) */ + if (H5_IS_BUFFER_OVERFLOW(image, 1, end)) + HGOTO_ERROR(H5E_FILE, H5E_OVERFLOW, NULL, "image pointer is out of bounds"); image++; udata->f->shared->sizeof_size = sblock->sizeof_size; /* Keep a local copy also */ /* Skip over reserved byte */ + if (H5_IS_BUFFER_OVERFLOW(image, 1, end)) + HGOTO_ERROR(H5E_FILE, H5E_OVERFLOW, NULL, "image pointer is out of bounds"); image++; /* Various B-tree sizes */ + if (H5_IS_BUFFER_OVERFLOW(image, 2, end)) + HGOTO_ERROR(H5E_FILE, H5E_OVERFLOW, NULL, "image pointer is out of bounds"); UINT16DECODE(image, sym_leaf_k); if (sym_leaf_k == 0) - HGOTO_ERROR(H5E_FILE, H5E_BADRANGE, NULL, "bad symbol table leaf node 1/2 rank") + HGOTO_ERROR(H5E_FILE, H5E_BADRANGE, NULL, "bad symbol table leaf node 1/2 rank"); udata->sym_leaf_k = sym_leaf_k; /* Keep a local copy also */ /* Need 'get' call to set other array values */ + if (H5_IS_BUFFER_OVERFLOW(image, 2, end)) + HGOTO_ERROR(H5E_FILE, H5E_OVERFLOW, NULL, "image pointer is out of bounds"); UINT16DECODE(image, snode_btree_k); if (snode_btree_k == 0) - HGOTO_ERROR(H5E_FILE, H5E_BADRANGE, NULL, "bad 1/2 rank for btree internal nodes") + HGOTO_ERROR(H5E_FILE, H5E_BADRANGE, NULL, "bad 1/2 rank for btree internal nodes"); udata->btree_k[H5B_SNODE_ID] = snode_btree_k; - /* - * Delay setting the value in the property list until we've checked + /* Delay setting the value in the property list until we've checked * for the indexed storage B-tree internal 'K' value later. */ /* File status flags (not really used yet) */ + if (H5_IS_BUFFER_OVERFLOW(image, 4, end)) + HGOTO_ERROR(H5E_FILE, H5E_OVERFLOW, NULL, "image pointer is out of bounds"); UINT32DECODE(image, status_flags); - HDassert(status_flags <= 255); + if (status_flags > 255) + HGOTO_ERROR(H5E_FILE, H5E_BADVALUE, NULL, "bad superblock status flags"); sblock->status_flags = (uint8_t)status_flags; if (sblock->status_flags & ~H5F_SUPER_ALL_FLAGS) - HGOTO_ERROR(H5E_FILE, H5E_BADVALUE, NULL, "bad flag value for superblock") + HGOTO_ERROR(H5E_FILE, H5E_BADVALUE, NULL, "bad flag value for superblock"); - /* - * If the superblock version # is greater than 0, read in the indexed + /* If the superblock version # is greater than 0, read in the indexed * storage B-tree internal 'K' value */ if (sblock->super_vers > HDF5_SUPERBLOCK_VERSION_DEF) { + if (H5_IS_BUFFER_OVERFLOW(image, 2, end)) + HGOTO_ERROR(H5E_FILE, H5E_OVERFLOW, NULL, "image pointer is out of bounds"); UINT16DECODE(image, chunk_btree_k); /* Reserved bytes are present only in version 1 */ - if (sblock->super_vers == HDF5_SUPERBLOCK_VERSION_1) - image += 2; /* reserved */ - } /* end if */ + if (sblock->super_vers == HDF5_SUPERBLOCK_VERSION_1) { + /* Reserved */ + if (H5_IS_BUFFER_OVERFLOW(image, 2, end)) + HGOTO_ERROR(H5E_FILE, H5E_OVERFLOW, NULL, "image pointer is out of bounds"); + image += 2; + } + } else chunk_btree_k = HDF5_BTREE_CHUNK_IK_DEF; udata->btree_k[H5B_CHUNK_ID] = chunk_btree_k; /* Remainder of "variable-sized" portion of superblock */ + if (H5_IS_BUFFER_OVERFLOW(image, H5F_sizeof_addr(udata->f) * 4, end)) + HGOTO_ERROR(H5E_FILE, H5E_OVERFLOW, NULL, "image pointer is out of bounds"); H5F_addr_decode(udata->f, (const uint8_t **)&image, &sblock->base_addr /*out*/); H5F_addr_decode(udata->f, (const uint8_t **)&image, &sblock->ext_addr /*out*/); H5F_addr_decode(udata->f, (const uint8_t **)&image, &udata->stored_eof /*out*/); H5F_addr_decode(udata->f, (const uint8_t **)&image, &sblock->driver_addr /*out*/); /* Allocate space for the root group symbol table entry */ - HDassert(!sblock->root_ent); + if (sblock->root_ent) + HGOTO_ERROR(H5E_FILE, H5E_BADVALUE, NULL, "root entry should not exist yet"); if (NULL == (sblock->root_ent = (H5G_entry_t *)H5MM_calloc(sizeof(H5G_entry_t)))) HGOTO_ERROR(H5E_FILE, H5E_CANTALLOC, NULL, - "can't allocate space for root group symbol table entry") + "can't allocate space for root group symbol table entry"); - /* decode the root group symbol table entry */ + /* Decode the root group symbol table entry */ if (H5G_ent_decode(udata->f, (const uint8_t **)&image, sblock->root_ent) < 0) - HGOTO_ERROR(H5E_FILE, H5E_CANTDECODE, NULL, "can't decode root group symbol table entry") + HGOTO_ERROR(H5E_FILE, H5E_CANTDECODE, NULL, "can't decode root group symbol table entry"); /* Set the root group address to the correct value */ sblock->root_addr = sblock->root_ent->header; @@ -533,26 +561,32 @@ H5F__cache_superblock_deserialize(const void *_image, size_t H5_ATTR_NDEBUG_UNUS /* Eliminate the driver info */ sblock->driver_addr = HADDR_UNDEF; udata->drvrinfo_removed = TRUE; - } /* end if */ + } /* NOTE: Driver info block is decoded separately, later */ - - } /* end if */ + } else { uint32_t read_chksum; /* Checksum read from file */ /* Skip over size of file addresses (already decoded) */ image++; udata->f->shared->sizeof_addr = sblock->sizeof_addr; /* Keep a local copy also */ - /* Skip over size of file sizes (already decoded) */ image++; udata->f->shared->sizeof_size = sblock->sizeof_size; /* Keep a local copy also */ + /* Check whether the image pointer is out of bounds */ + if (H5_IS_BUFFER_OVERFLOW(image, 1, end)) + HGOTO_ERROR(H5E_FILE, H5E_OVERFLOW, NULL, "image pointer is out of bounds"); + /* File status flags (not really used yet) */ sblock->status_flags = *image++; if (sblock->status_flags & ~H5F_SUPER_ALL_FLAGS) - HGOTO_ERROR(H5E_FILE, H5E_BADVALUE, NULL, "bad flag value for superblock") + HGOTO_ERROR(H5E_FILE, H5E_BADVALUE, NULL, "bad flag value for superblock"); + + /* Check whether the image pointer will be out of bounds */ + if (H5_IS_BUFFER_OVERFLOW(image, H5F_SIZEOF_ADDR(udata->f) * 4, end)) + HGOTO_ERROR(H5E_FILE, H5E_OVERFLOW, NULL, "image pointer is out of bounds"); /* Base, superblock extension, end of file & root group object header addresses */ H5F_addr_decode(udata->f, (const uint8_t **)&image, &sblock->base_addr /*out*/); @@ -562,6 +596,10 @@ H5F__cache_superblock_deserialize(const void *_image, size_t H5_ATTR_NDEBUG_UNUS /* checksum verification already done in verify_chksum cb */ + /* Check whether the image pointer will be out of bounds */ + if (H5_IS_BUFFER_OVERFLOW(image, sizeof(uint32_t), end)) + HGOTO_ERROR(H5E_FILE, H5E_OVERFLOW, NULL, "image pointer is out of bounds"); + /* Decode checksum */ UINT32DECODE(image, read_chksum); @@ -571,19 +609,19 @@ H5F__cache_superblock_deserialize(const void *_image, size_t H5_ATTR_NDEBUG_UNUS * any attempt to load the Driver Information Block. */ sblock->driver_addr = HADDR_UNDEF; - } /* end else */ + } - /* Sanity check */ - HDassert((size_t)(image - (const uint8_t *)_image) <= len); + /* Size check */ + if ((size_t)(image - (const uint8_t *)_image) > len) + HDONE_ERROR(H5E_FILE, H5E_BADVALUE, NULL, "bad decoded superblock size"); - /* Set return value */ ret_value = sblock; done: /* Release the [possibly partially initialized] superblock on error */ if (!ret_value && sblock) if (H5F__super_free(sblock) < 0) - HDONE_ERROR(H5E_FILE, H5E_CANTFREE, NULL, "unable to destroy superblock data") + HDONE_ERROR(H5E_FILE, H5E_CANTFREE, NULL, "unable to destroy superblock data"); FUNC_LEAVE_NOAPI(ret_value) } /* end H5F__cache_superblock_deserialize() */ diff --git a/src/H5Gint.c b/src/H5Gint.c index e388b326a40..eb983cdad52 100644 --- a/src/H5Gint.c +++ b/src/H5Gint.c @@ -909,15 +909,13 @@ H5G__visit_cb(const H5O_link_t *lnk, void *_udata) /* Check if we've seen the object the link references before */ if (NULL == H5SL_search(udata->visited, &obj_pos)) { H5O_type_t otype; /* Basic object type (group, dataset, etc.) */ - unsigned rc; /* Reference count of object */ /* Get the object's reference count and type */ - if (H5O_get_rc_and_type(&obj_oloc, &rc, &otype) < 0) + if (H5O_get_rc_and_type(&obj_oloc, NULL, &otype) < 0) HGOTO_ERROR(H5E_SYM, H5E_CANTGET, H5_ITER_ERROR, "unable to get object info") - /* If its ref count is > 1, we add it to the list of visited objects */ - /* (because it could come up again during traversal) */ - if (rc > 1) { + /* Add it to the list of visited objects */ + { H5_obj_t *new_node; /* New object node for visited list */ /* Allocate new object "position" node */ @@ -931,7 +929,7 @@ H5G__visit_cb(const H5O_link_t *lnk, void *_udata) if (H5SL_insert(udata->visited, new_node, new_node) < 0) HGOTO_ERROR(H5E_SYM, H5E_CANTINSERT, H5_ITER_ERROR, "can't insert object node into visited list") - } /* end if */ + } /* If it's a group, we recurse into it */ if (otype == H5O_TYPE_GROUP) { @@ -1026,7 +1024,6 @@ H5G_visit(H5G_loc_t *loc, const char *group_name, H5_index_t idx_type, H5_iter_o hid_t gid = H5I_INVALID_HID; /* Group ID */ H5G_t *grp = NULL; /* Group opened */ H5G_loc_t start_loc; /* Location of starting group */ - unsigned rc; /* Reference count of object */ herr_t ret_value = FAIL; /* Return value */ /* Portably clear udata struct (before FUNC_ENTER) */ @@ -1068,13 +1065,8 @@ H5G_visit(H5G_loc_t *loc, const char *group_name, H5_index_t idx_type, H5_iter_o if ((udata.visited = H5SL_create(H5SL_TYPE_OBJ, NULL)) == NULL) HGOTO_ERROR(H5E_SYM, H5E_CANTCREATE, FAIL, "can't create skip list for visited objects") - /* Get the group's reference count */ - if (H5O_get_rc_and_type(&grp->oloc, &rc, NULL) < 0) - HGOTO_ERROR(H5E_SYM, H5E_CANTGET, FAIL, "unable to get object info") - - /* If its ref count is > 1, we add it to the list of visited objects */ - /* (because it could come up again during traversal) */ - if (rc > 1) { + /* Add it to the list of visited objects */ + { H5_obj_t *obj_pos; /* New object node for visited list */ /* Allocate new object "position" node */ @@ -1088,7 +1080,7 @@ H5G_visit(H5G_loc_t *loc, const char *group_name, H5_index_t idx_type, H5_iter_o /* Add to list of visited objects */ if (H5SL_insert(udata.visited, obj_pos, obj_pos) < 0) HGOTO_ERROR(H5E_SYM, H5E_CANTINSERT, FAIL, "can't insert object node into visited list") - } /* end if */ + } /* Attempt to get the link info for this group */ if ((linfo_exists = H5G__obj_get_linfo(&(grp->oloc), &linfo)) < 0) diff --git a/src/H5HLcache.c b/src/H5HLcache.c index d7c7ce865dd..dcf1249ddaa 100644 --- a/src/H5HLcache.c +++ b/src/H5HLcache.c @@ -81,7 +81,8 @@ static herr_t H5HL__cache_datablock_notify(H5C_notify_action_t action, void *_th static herr_t H5HL__cache_datablock_free_icr(void *thing); /* Header deserialization */ -static herr_t H5HL__hdr_deserialize(H5HL_t *heap, const uint8_t *image, H5HL_cache_prfx_ud_t *udata); +static herr_t H5HL__hdr_deserialize(H5HL_t *heap, const uint8_t *image, size_t len, + H5HL_cache_prfx_ud_t *udata); /* Free list de/serialization */ static herr_t H5HL__fl_deserialize(H5HL_t *heap); @@ -148,9 +149,10 @@ const H5AC_class_t H5AC_LHEAP_DBLK[1] = {{ *------------------------------------------------------------------------- */ static herr_t -H5HL__hdr_deserialize(H5HL_t *heap, const uint8_t *image, H5HL_cache_prfx_ud_t *udata) +H5HL__hdr_deserialize(H5HL_t *heap, const uint8_t *image, size_t len, H5HL_cache_prfx_ud_t *udata) { - herr_t ret_value = SUCCEED; /* Return value */ + const uint8_t *p_end = image + len - 1; /* End of image buffer */ + herr_t ret_value = SUCCEED; FUNC_ENTER_STATIC @@ -159,16 +161,22 @@ H5HL__hdr_deserialize(H5HL_t *heap, const uint8_t *image, H5HL_cache_prfx_ud_t * HDassert(image); HDassert(udata); - /* Check magic number */ + /* Magic number */ + if (H5_IS_BUFFER_OVERFLOW(image, H5_SIZEOF_MAGIC, p_end)) + HGOTO_ERROR(H5E_HEAP, H5E_OVERFLOW, FAIL, "ran off end of input buffer while decoding"); if (HDmemcmp(image, H5HL_MAGIC, (size_t)H5_SIZEOF_MAGIC) != 0) - HGOTO_ERROR(H5E_HEAP, H5E_BADVALUE, FAIL, "bad local heap signature") + HGOTO_ERROR(H5E_HEAP, H5E_BADVALUE, FAIL, "bad local heap signature"); image += H5_SIZEOF_MAGIC; /* Version */ + if (H5_IS_BUFFER_OVERFLOW(image, 1, p_end)) + HGOTO_ERROR(H5E_HEAP, H5E_OVERFLOW, FAIL, "ran off end of input buffer while decoding"); if (H5HL_VERSION != *image++) - HGOTO_ERROR(H5E_HEAP, H5E_VERSION, FAIL, "wrong version number in local heap") + HGOTO_ERROR(H5E_HEAP, H5E_VERSION, FAIL, "wrong version number in local heap"); /* Reserved */ + if (H5_IS_BUFFER_OVERFLOW(image, 3, p_end)) + HGOTO_ERROR(H5E_HEAP, H5E_OVERFLOW, FAIL, "ran off end of input buffer while decoding"); image += 3; /* Store the prefix's address & length */ @@ -176,16 +184,28 @@ H5HL__hdr_deserialize(H5HL_t *heap, const uint8_t *image, H5HL_cache_prfx_ud_t * heap->prfx_size = udata->sizeof_prfx; /* Heap data size */ + if (H5_IS_BUFFER_OVERFLOW(image, udata->sizeof_size, p_end)) + HGOTO_ERROR(H5E_HEAP, H5E_OVERFLOW, FAIL, "ran off end of input buffer while decoding"); H5F_DECODE_LENGTH_LEN(image, heap->dblk_size, udata->sizeof_size); /* Free list head */ + if (H5_IS_BUFFER_OVERFLOW(image, udata->sizeof_size, p_end)) + HGOTO_ERROR(H5E_HEAP, H5E_OVERFLOW, FAIL, "ran off end of input buffer while decoding"); H5F_DECODE_LENGTH_LEN(image, heap->free_block, udata->sizeof_size); if (heap->free_block != H5HL_FREE_NULL && heap->free_block >= heap->dblk_size) - HGOTO_ERROR(H5E_HEAP, H5E_BADVALUE, FAIL, "bad heap free list") + HGOTO_ERROR(H5E_HEAP, H5E_BADVALUE, FAIL, "bad heap free list"); /* Heap data address */ + if (H5_IS_BUFFER_OVERFLOW(image, udata->sizeof_addr, p_end)) + HGOTO_ERROR(H5E_HEAP, H5E_OVERFLOW, FAIL, "ran off end of input buffer while decoding"); H5F_addr_decode_len(udata->sizeof_addr, &image, &(heap->dblk_addr)); + /* Check that the datablock address is valid (might not be true + * in a corrupt file) + */ + if (!H5F_addr_defined(heap->dblk_addr)) + HGOTO_ERROR(H5E_HEAP, H5E_BADVALUE, FAIL, "bad datablock address"); + done: FUNC_LEAVE_NOAPI(ret_value) } /* end H5HL__hdr_deserialize() */ @@ -222,11 +242,11 @@ H5HL__fl_deserialize(H5HL_t *heap) /* Sanity check */ if (free_block >= heap->dblk_size) - HGOTO_ERROR(H5E_HEAP, H5E_BADRANGE, FAIL, "bad heap free list") + HGOTO_ERROR(H5E_HEAP, H5E_BADRANGE, FAIL, "bad heap free list"); /* Allocate & initialize free list node */ if (NULL == (fl = H5FL_MALLOC(H5HL_free_t))) - HGOTO_ERROR(H5E_HEAP, H5E_CANTALLOC, FAIL, "memory allocation failed") + HGOTO_ERROR(H5E_HEAP, H5E_CANTALLOC, FAIL, "memory allocation failed"); fl->offset = (size_t)free_block; fl->prev = tail; fl->next = NULL; @@ -235,12 +255,12 @@ H5HL__fl_deserialize(H5HL_t *heap) image = heap->dblk_image + free_block; H5F_DECODE_LENGTH_LEN(image, free_block, heap->sizeof_size); if (0 == free_block) - HGOTO_ERROR(H5E_HEAP, H5E_BADVALUE, FAIL, "free block size is zero?") + HGOTO_ERROR(H5E_HEAP, H5E_BADVALUE, FAIL, "free block size is zero?"); /* Decode length of this free block */ H5F_DECODE_LENGTH_LEN(image, fl->size, heap->sizeof_size); if ((fl->offset + fl->size) > heap->dblk_size) - HGOTO_ERROR(H5E_HEAP, H5E_BADRANGE, FAIL, "bad heap free list") + HGOTO_ERROR(H5E_HEAP, H5E_BADRANGE, FAIL, "bad heap free list"); /* Append node onto list */ if (tail) @@ -344,8 +364,7 @@ H5HL__cache_prefix_get_initial_load_size(void H5_ATTR_UNUSED *_udata, size_t *im *------------------------------------------------------------------------- */ static herr_t -H5HL__cache_prefix_get_final_load_size(const void *_image, size_t H5_ATTR_NDEBUG_UNUSED image_len, - void *_udata, size_t *actual_len) +H5HL__cache_prefix_get_final_load_size(const void *_image, size_t image_len, void *_udata, size_t *actual_len) { const uint8_t *image = (const uint8_t *)_image; /* Pointer into raw data buffer */ H5HL_cache_prfx_ud_t *udata = (H5HL_cache_prfx_ud_t *)_udata; /* User data for callback */ @@ -361,8 +380,8 @@ H5HL__cache_prefix_get_final_load_size(const void *_image, size_t H5_ATTR_NDEBUG HDassert(*actual_len == image_len); /* Deserialize the heap's header */ - if (H5HL__hdr_deserialize(&heap, (const uint8_t *)image, udata) < 0) - HGOTO_ERROR(H5E_HEAP, H5E_CANTDECODE, FAIL, "can't decode local heap header") + if (H5HL__hdr_deserialize(&heap, (const uint8_t *)image, image_len, udata) < 0) + HGOTO_ERROR(H5E_HEAP, H5E_CANTDECODE, FAIL, "can't decode local heap header"); /* Set the final size for the cache image */ *actual_len = heap.prfx_size; @@ -394,12 +413,12 @@ H5HL__cache_prefix_get_final_load_size(const void *_image, size_t H5_ATTR_NDEBUG *------------------------------------------------------------------------- */ static void * -H5HL__cache_prefix_deserialize(const void *_image, size_t H5_ATTR_NDEBUG_UNUSED len, void *_udata, - hbool_t H5_ATTR_UNUSED *dirty) +H5HL__cache_prefix_deserialize(const void *_image, size_t len, void *_udata, hbool_t H5_ATTR_UNUSED *dirty) { H5HL_t *heap = NULL; /* Local heap */ H5HL_prfx_t *prfx = NULL; /* Heap prefix deserialized */ const uint8_t *image = (const uint8_t *)_image; /* Pointer into decoding buffer */ + const uint8_t *p_end = image + len - 1; /* End of image buffer */ H5HL_cache_prfx_ud_t *udata = (H5HL_cache_prfx_ud_t *)_udata; /* User data for callback */ void *ret_value = NULL; /* Return value */ @@ -420,8 +439,8 @@ H5HL__cache_prefix_deserialize(const void *_image, size_t H5_ATTR_NDEBUG_UNUSED HGOTO_ERROR(H5E_HEAP, H5E_CANTALLOC, NULL, "can't allocate local heap structure"); /* Deserialize the heap's header */ - if (H5HL__hdr_deserialize(heap, (const uint8_t *)image, udata) < 0) - HGOTO_ERROR(H5E_HEAP, H5E_CANTDECODE, NULL, "can't decode local heap header") + if (H5HL__hdr_deserialize(heap, (const uint8_t *)image, len, udata) < 0) + HGOTO_ERROR(H5E_HEAP, H5E_CANTDECODE, NULL, "can't decode local heap header"); /* Allocate the heap prefix */ if (NULL == (prfx = H5HL__prfx_new(heap))) @@ -436,7 +455,7 @@ H5HL__cache_prefix_deserialize(const void *_image, size_t H5_ATTR_NDEBUG_UNUSED /* Allocate space for the heap data image */ if (NULL == (heap->dblk_image = H5FL_BLK_MALLOC(lheap_chunk, heap->dblk_size))) - HGOTO_ERROR(H5E_HEAP, H5E_CANTALLOC, NULL, "memory allocation failed") + HGOTO_ERROR(H5E_HEAP, H5E_CANTALLOC, NULL, "memory allocation failed"); /* Set image to the start of the data block. This is necessary * because there may be a gap between the used portion of the @@ -444,11 +463,13 @@ H5HL__cache_prefix_deserialize(const void *_image, size_t H5_ATTR_NDEBUG_UNUSED image = ((const uint8_t *)_image) + heap->prfx_size; /* Copy the heap data from the speculative read buffer */ + if (H5_IS_BUFFER_OVERFLOW(image, heap->dblk_size, p_end)) + HGOTO_ERROR(H5E_HEAP, H5E_OVERFLOW, NULL, "ran off end of input buffer while decoding"); H5MM_memcpy(heap->dblk_image, image, heap->dblk_size); /* Build free list */ if (H5HL__fl_deserialize(heap) < 0) - HGOTO_ERROR(H5E_HEAP, H5E_CANTINIT, NULL, "can't initialize free list") + HGOTO_ERROR(H5E_HEAP, H5E_CANTINIT, NULL, "can't initialize free list"); } else /* Note that the heap should _NOT_ be a single @@ -538,7 +559,6 @@ H5HL__cache_prefix_serialize(const H5_ATTR_NDEBUG_UNUSED H5F_t *f, void *_image, H5HL_prfx_t *prfx = (H5HL_prfx_t *)_thing; /* Pointer to local heap prefix to query */ H5HL_t *heap; /* Pointer to the local heap */ uint8_t *image = (uint8_t *)_image; /* Pointer into image buffer */ - size_t buf_size; /* expected size of the image buffer */ FUNC_ENTER_STATIC_NOERR @@ -555,11 +575,13 @@ H5HL__cache_prefix_serialize(const H5_ATTR_NDEBUG_UNUSED H5F_t *f, void *_image, heap = prfx->heap; HDassert(heap); +#ifndef NDEBUG /* Compute the buffer size */ - buf_size = heap->prfx_size; + size_t buf_size = heap->prfx_size; /* expected size of the image buffer */ if (heap->single_cache_obj) buf_size += heap->dblk_size; HDassert(len == buf_size); +#endif /* Update the free block value from the free list */ heap->free_block = heap->freelist ? heap->freelist->offset : H5HL_FREE_NULL; @@ -647,7 +669,7 @@ H5HL__cache_prefix_free_icr(void *_thing) /* Destroy local heap prefix */ if (H5HL__prfx_dest(prfx) < 0) - HGOTO_ERROR(H5E_HEAP, H5E_CANTRELEASE, FAIL, "can't destroy local heap prefix") + HGOTO_ERROR(H5E_HEAP, H5E_CANTRELEASE, FAIL, "can't destroy local heap prefix"); done: FUNC_LEAVE_NOAPI(ret_value) @@ -865,7 +887,7 @@ H5HL__cache_datablock_notify(H5C_notify_action_t action, void *_thing) /* Pin the heap's prefix */ if (FAIL == H5AC_pin_protected_entry(dblk->heap->prfx)) - HGOTO_ERROR(H5E_HEAP, H5E_CANTPIN, FAIL, "unable to pin local heap prefix") + HGOTO_ERROR(H5E_HEAP, H5E_CANTPIN, FAIL, "unable to pin local heap prefix"); break; case H5AC_NOTIFY_ACTION_AFTER_FLUSH: @@ -885,11 +907,11 @@ H5HL__cache_datablock_notify(H5C_notify_action_t action, void *_thing) /* Unpin the local heap prefix */ if (FAIL == H5AC_unpin_entry(dblk->heap->prfx)) - HGOTO_ERROR(H5E_HEAP, H5E_CANTUNPIN, FAIL, "unable to unpin local heap prefix") + HGOTO_ERROR(H5E_HEAP, H5E_CANTUNPIN, FAIL, "unable to unpin local heap prefix"); break; default: - HGOTO_ERROR(H5E_ARGS, H5E_BADVALUE, FAIL, "unknown action from metadata cache") + HGOTO_ERROR(H5E_ARGS, H5E_BADVALUE, FAIL, "unknown action from metadata cache"); break; } @@ -929,7 +951,7 @@ H5HL__cache_datablock_free_icr(void *_thing) /* Destroy the data block */ if (H5HL__dblk_dest(dblk) < 0) - HGOTO_ERROR(H5E_HEAP, H5E_CANTFREE, FAIL, "unable to destroy local heap data block") + HGOTO_ERROR(H5E_HEAP, H5E_CANTFREE, FAIL, "unable to destroy local heap data block"); done: FUNC_LEAVE_NOAPI(ret_value) diff --git a/src/H5Omessage.c b/src/H5Omessage.c index 8b065865a43..8c2d4de4f95 100644 --- a/src/H5Omessage.c +++ b/src/H5Omessage.c @@ -675,12 +675,11 @@ H5O_msg_free_real(const H5O_msg_class_t *type, void *msg_native) { FUNC_ENTER_NOAPI_NOINIT_NOERR - /* check args */ - HDassert(type); + /* Don't assert on args since this could be called in cleanup code */ if (msg_native) { H5O__msg_reset_real(type, msg_native); - if (NULL != (type->free)) + if (type && type->free) (type->free)(msg_native); else H5MM_xfree(msg_native); diff --git a/tools/src/h5dump/h5dump_ddl.c b/tools/src/h5dump/h5dump_ddl.c index e45a68fab7f..9f96af3b0e0 100644 --- a/tools/src/h5dump/h5dump_ddl.c +++ b/tools/src/h5dump/h5dump_ddl.c @@ -853,10 +853,7 @@ dump_group(hid_t gid, const char *name) H5Oget_info3(gid, &oinfo, H5O_INFO_BASIC); - /* Must check for uniqueness of all objects if we've traversed an elink, - * otherwise only check if the reference count > 1. - */ - if (oinfo.rc > 1 || hit_elink) { + { obj_t *found_obj; /* Found object */ found_obj = search_obj(group_table, &oinfo.token); @@ -880,10 +877,6 @@ dump_group(hid_t gid, const char *name) link_iteration(gid, crt_order_flags); } } - else { - attr_iteration(gid, attr_crt_order_flags); - link_iteration(gid, crt_order_flags); - } dump_indent -= COL; ctx.indent_level--; diff --git a/tools/testfiles/tgroup-2.ddl b/tools/testfiles/tgroup-2.ddl index 2ac8ac6ef5b..5374742a73a 100644 --- a/tools/testfiles/tgroup-2.ddl +++ b/tools/testfiles/tgroup-2.ddl @@ -17,14 +17,7 @@ GROUP "/" { } } GROUP "g2" { - GROUP "g2.1" { - GROUP "g2.1.1" { - } - GROUP "g2.1.2" { - } - GROUP "g2.1.3" { - } - } + HARDLINK "/g2" } GROUP "g3" { GROUP "g3.1" {