Skip to content

Commit

Permalink
Brings the following changesets over from develop: (#2328)
Browse files Browse the repository at this point in the history 
b9244a8 Align arg types of H5D_chunk_iter_op_t with H5Dget_chunk_info (#2074)
70cf2c3 Removed idioms and misc. text clean-up (#2320)
8102fa8 Only document Fortran functions (#2319)
784061b moved onion VFD to FAPL group (#2321)
6b6bcde Hdffv 11052 (#2315)
10c693a Update hdf5_header.html
0cb5808 Hdffv 11052 (#2303)
a1c81ed added doc. warning for H5Literate_async return value (#2295)
502b32b Updated H5ES documenation (#2293)
a903600 Fix for HDFFV-11052: h5debug fails on a corrupted file (h5_nrefs_POC)… (#2291)
  • Loading branch information
@derobins
derobins authored Dec 21, 2022
1 parent a2ce031 commit 83c7f92
Show file tree
Hide file tree
Showing 17 changed files with 153 additions and 66 deletions.
10 changes: 9 additions & 1 deletion doxygen/Doxyfile.in
View file
Original file line number Diff line number Diff line change
Expand Up @@ -897,7 +897,15 @@ EXCLUDE_SYMLINKS = NO
# Note that the wildcards are matched against the file with absolute path, so to
# exclude all test directories for example use the pattern */test/*

EXCLUDE_PATTERNS =
EXCLUDE_PATTERNS = */fortran/test/*
EXCLUDE_PATTERNS += */fortran/testpar/*
EXCLUDE_PATTERNS += */fortran/examples/*
EXCLUDE_PATTERNS += */fortran/src/*.c
EXCLUDE_PATTERNS += */fortran/src/*.h
EXCLUDE_PATTERNS += */hl/fortran/examples/*
EXCLUDE_PATTERNS += */hl/fortran/test/*
EXCLUDE_PATTERNS += */hl/fortran/src/*.c
EXCLUDE_PATTERNS += */hl/fortran/src/*.h

# The EXCLUDE_SYMBOLS tag can be used to specify one or more symbol names
# (namespaces, classes, functions, etc.) that should be excluded from the
Expand Down
13 changes: 4 additions & 9 deletions doxygen/dox/ReferenceManual.dox
View file
Original file line number Diff line number Diff line change
Expand Up @@ -145,31 +145,26 @@ Functions with \ref ASYNC<br />
</tr>
</table>

</td></tr>
<tr><th>Mind the gap</th></tr>
<tr><td>
Follow these simple rules and stay out of trouble:

\li \Bold{Handle discipline:} The HDF5 C-API is rife with handles or
\li \Bold{Handle discipline:} The HDF5 API is rife with handles or
identifiers, which you typically obtain by creating new HDF5 items, copying
items, or retrieving facets of items. \Emph{You acquire a handle, you own it!}
(Colin Powell) In other words, you are responsible for releasing the underlying
items, or retrieving facets of items. Consequently, \Bold{and most importantly}, you are
responsible for releasing the underlying
resources via the matching \Code{H5*close()} call, or deal with the consequences
of resource leakage.
\li \Bold{Closed means closed:} Do not pass identifiers that were previously
\Code{H5*close()}-d to other API functions! It will generate an error.
\li \Bold{Dynamic memory allocation:} The API contains a few functions in which the
HDF5 library dynamically allocates memory on the caller's behalf. The caller owns
this memory and eventually must free it by calling H5free_memory(). (\Bold{Not}
the `free` function \Emph{du jour}!)
this memory and eventually must free it by calling H5free_memory() and not language-explicit memory functions.
\li \Bold{Be careful with that saw:} Do not modify the underlying collection when an
iteration is in progress!
\li \Bold{Use of locations:} Certain API functions, typically called \Code{H5***_by_name}
use a combination of identifiers and path names to refer to HDF5 objects.
If the identifier fully specifies the object in question, pass \Code{'.'} (a dot)
for the name!

Break a leg!
</td>
</tr>
</table>
Expand Down
2 changes: 1 addition & 1 deletion doxygen/hdf5_header.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
</head>
<body>

<div style="background:#FFDDDD;font-size:120%;text-align:center;margin:0;padding:5px">Please, help us to better know about our user community by answering the following short survey: <a href="https://www.hdfgroup.org/website-survey/">https://www.hdfgroup.org/website-survey/</a></div>
<div style="background:#FFDDDD;font-size:120%;text-align:center;margin:0;padding:5px">Please, help us to better serve our user community by answering the following short survey: <a href="https://www.hdfgroup.org/website-survey/">https://www.hdfgroup.org/website-survey/</a></div>

<div id="top"><!-- do not remove this div, it is closed by doxygen! -->

Expand Down
19 changes: 19 additions & 0 deletions release_docs/RELEASE.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,25 @@ Bug Fixes since HDF5-1.13.3 release
===================================
Library
-------
- Seg fault on file close

h5debug fails at file close with core dump on a file that has an
illegal file size in its cache image. In H5F_dest(), the library
performs all the closing operations for the file and keeps track of
the error encountered when reading the file cache image.
At the end of the routine, it frees the file's file structure and
returns error. Due to the error return, the file object is not removed
from the ID node table. This eventually causes assertion failure in
H5VL__native_file_close() when the library finally exits and tries to
access that file object in the table for closing.

The closing routine, H5F_dest(), will not free the file structure if
there is error, keeping a valid file structure in the ID node table.
It will be freed later in H5VL__native_file_close() when the
library exits and terminates the file package.

(VC - 2022/12/14, HDFFV-11052, CVE-2020-10812)

- Fix CVE-2018-13867 / GHSA-j8jr-chrh-qfrf

Validate location (offset) of the accumulated metadata when comparing.
Expand Down
16 changes: 8 additions & 8 deletions src/H5D.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2515,18 +2515,18 @@ H5Dget_chunk_info_by_coord(hid_t dset_id, const hsize_t *offset, unsigned *filte
*
* typedef int (*H5D_chunk_iter_op_t)(
* const hsize_t *offset,
* uint32_t filter_mask,
* unsigned filter_mask,
* haddr_t addr,
* uint32_t nbytes,
* hsize_t size,
* void *op_data);
*
* H5D_chunk_iter_op_t parameters:
* hsize_t *offset; IN/OUT: Array of starting logical coordinates of chunk.
* uint32_t filter_mask; IN: Filter mask of chunk.
* haddr_t addr; IN: Offset in file of chunk data.
* uint32_t nbytes; IN: Size in number of bytes of chunk data in file.
* void *op_data; IN/OUT: Pointer to any user-defined data
* associated with the operation.
* hsize_t *offset; IN/OUT: Logical position of the chunk’s first element in units of dataset
* elements
* unsigned filter_mask; IN: Bitmask indicating the filters used when the chunk was written haddr_t
* addr; IN: Chunk address in the file
* hsize_t; IN: Chunk size in bytes, 0 if the chunk does not exist
* void *op_data; IN/OUT: Pointer to any user-defined data associated with the operation.
*
* The return values from an operator are:
* Zero (H5_ITER_CONT) causes the iterator to continue, returning zero when all
Expand Down
4 changes: 2 additions & 2 deletions src/H5Dchunk.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8278,8 +8278,8 @@ H5D__chunk_iter_cb(const H5D_chunk_rec_t *chunk_rec, void *udata)
FUNC_ENTER_PACKAGE_NOERR

/* Check for callback failure and pass along return value */
if ((ret_value = (data->op)(offset, chunk_rec->filter_mask, chunk_rec->chunk_addr, chunk_rec->nbytes,
data->op_data)) < 0)
if ((ret_value = (data->op)(offset, (unsigned)chunk_rec->filter_mask, chunk_rec->chunk_addr,
(hsize_t)chunk_rec->nbytes, data->op_data)) < 0)
HERROR(H5E_DATASET, H5E_CANTNEXT, "iteration operator failed");

FUNC_LEAVE_NOAPI(ret_value)
Expand Down
2 changes: 1 addition & 1 deletion src/H5Dpublic.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,7 @@ typedef herr_t (*H5D_gather_func_t)(const void *dst_buf, size_t dst_buf_bytes_us
* \li A negative (#H5_ITER_ERROR) causes the iterator to immediately
* return that value, indicating failure.
*/
typedef int (*H5D_chunk_iter_op_t)(const hsize_t *offset, uint32_t filter_mask, haddr_t addr, uint32_t size,
typedef int (*H5D_chunk_iter_op_t)(const hsize_t *offset, unsigned filter_mask, haddr_t addr, hsize_t size,
void *op_data);
//! <!-- [H5D_chunk_iter_op_t_snip] -->

Expand Down
10 changes: 5 additions & 5 deletions src/H5ESpublic.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ H5_DLL herr_t H5ESwait(hid_t es_id, uint64_t timeout, size_t *num_in_progress, h
* \param[out] err_occurred Status indicating if error is present in the event set
* \returns \herr_t
*
* \details H5ESget_count() attempts to cancel operations in an event set specified
* \details H5EScancel() attempts to cancel operations in an event set specified
* by \p es_id. H5ES_NONE is a valid value for \p es_id, but functions as a no-op.
*
* \since 1.13.0
Expand Down Expand Up @@ -217,14 +217,14 @@ H5_DLL herr_t H5ESget_count(hid_t es_id, size_t *count);
/**
* \ingroup H5ES
*
* \brief Retrieves the next operation counter to be assigned in an event set
* \brief Retrieves the accumulative operation counter for an event set
*
* \es_id
* \param[out] counter The next counter value to be assigned to an event
* \param[out] counter The accumulative counter value for an event set
* \returns \herr_t
*
* \details H5ESget_op_counter() retrieves the \p counter that will be assigned
* to the next operation inserted into the event set \p es_id.
* \details H5ESget_op_counter() retrieves the current accumulative count of
* event set operations since the event set creation of \p es_id.
*
* \note This is designed for wrapper libraries mainly, to use as a mechanism
* for matching operations inserted into the event set with possible
Expand Down
8 changes: 4 additions & 4 deletions src/H5FDonion.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -142,11 +142,11 @@ H5_DLL hid_t H5FD_onion_init(void);

/**
* --------------------------------------------------------------------------
* \ingroup H5P
* \ingroup FAPL
*
* \brief get the onion info from the file access property list
*
* \param[in] fapl_id The ID of the file access property list
* \fapl_id
* \param[out] fa_out The pointer to the structure H5FD_onion_fapl_info_t
*
* \return \herr_t
Expand All @@ -159,11 +159,11 @@ H5_DLL herr_t H5Pget_fapl_onion(hid_t fapl_id, H5FD_onion_fapl_info_t *fa_out);

/**
* --------------------------------------------------------------------------
* \ingroup H5P
* \ingroup FAPL
*
* \brief set the onion info for the file access property list
*
* \param[in] fapl_id The ID of the file access property list
* \fapl_id
* \param[in] fa The pointer to the structure H5FD_onion_fapl_info_t
*
* \return \herr_t
Expand Down
4 changes: 3 additions & 1 deletion src/H5Fint.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1615,7 +1615,9 @@ H5F__dest(H5F_t *f, hbool_t flush)
if (H5FO_top_dest(f) < 0)
HDONE_ERROR(H5E_FILE, H5E_CANTINIT, FAIL, "problems closing file")
f->shared = NULL;
f = H5FL_FREE(H5F_t, f);

if (ret_value >= 0)
f = H5FL_FREE(H5F_t, f);

FUNC_LEAVE_NOAPI(ret_value)
} /* end H5F__dest() */
Expand Down
5 changes: 5 additions & 0 deletions src/H5Lpublic.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -923,6 +923,11 @@ H5_DLL herr_t H5Literate2(hid_t grp_id, H5_index_t idx_type, H5_iter_order_t ord
/**
* --------------------------------------------------------------------------
* \ingroup ASYNC
*
* \warning The returned value of the callback routine op will not be set
* in the return value for H5Literate_async(), so the \p herr_t value
* should not be used for determining the return state of the callback routine.
*
* \async_variant_of{H5Literate}
*/
#ifndef H5_DOXYGEN
Expand Down
52 changes: 29 additions & 23 deletions src/H5VLnative_file.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -753,29 +753,35 @@ H5VL__native_file_close(void *file, hid_t H5_ATTR_UNUSED dxpl_id, void H5_ATTR_U
FUNC_ENTER_PACKAGE

/* This routine should only be called when a file ID's ref count drops to zero */
HDassert(H5F_ID_EXISTS(f));

/* Flush file if this is the last reference to this id and we have write
* intent, unless it will be flushed by the "shared" file being closed.
* This is only necessary to replicate previous behaviour, and could be
* disabled by an option/property to improve performance.
*/
if ((H5F_NREFS(f) > 1) && (H5F_INTENT(f) & H5F_ACC_RDWR)) {
/* Get the file ID corresponding to the H5F_t struct */
if (H5I_find_id(f, H5I_FILE, &file_id) < 0 || H5I_INVALID_HID == file_id)
HGOTO_ERROR(H5E_ID, H5E_CANTGET, FAIL, "invalid ID")

/* Get the number of references outstanding for this file ID */
if ((nref = H5I_get_ref(file_id, FALSE)) < 0)
HGOTO_ERROR(H5E_ID, H5E_CANTGET, FAIL, "can't get ID ref count")
if (nref == 1)
if (H5F__flush(f) < 0)
HGOTO_ERROR(H5E_FILE, H5E_CANTFLUSH, FAIL, "unable to flush cache")
} /* end if */

/* Close the file */
if (H5F__close(f) < 0)
HGOTO_ERROR(H5E_FILE, H5E_CANTDEC, FAIL, "can't close file")
HDassert(f->shared == NULL || H5F_ID_EXISTS(f));

if (f->shared == NULL)
f = H5FL_FREE(H5F_t, f);

else {

/* Flush file if this is the last reference to this id and we have write
* intent, unless it will be flushed by the "shared" file being closed.
* This is only necessary to replicate previous behaviour, and could be
* disabled by an option/property to improve performance.
*/
if ((H5F_NREFS(f) > 1) && (H5F_INTENT(f) & H5F_ACC_RDWR)) {
/* Get the file ID corresponding to the H5F_t struct */
if (H5I_find_id(f, H5I_FILE, &file_id) < 0 || H5I_INVALID_HID == file_id)
HGOTO_ERROR(H5E_ID, H5E_CANTGET, FAIL, "invalid ID")

/* Get the number of references outstanding for this file ID */
if ((nref = H5I_get_ref(file_id, FALSE)) < 0)
HGOTO_ERROR(H5E_ID, H5E_CANTGET, FAIL, "can't get ID ref count")
if (nref == 1)
if (H5F__flush(f) < 0)
HGOTO_ERROR(H5E_FILE, H5E_CANTFLUSH, FAIL, "unable to flush cache")
} /* end if */

/* Close the file */
if (H5F__close(f) < 0)
HGOTO_ERROR(H5E_FILE, H5E_CANTDEC, FAIL, "can't close file")
}

done:
FUNC_LEAVE_NOAPI(ret_value)
Expand Down
1 change: 1 addition & 0 deletions test/CMakeTests.cmake
View file
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ set (HDF5_REFERENCE_TEST_FILES
btree_idx_1_6.h5
btree_idx_1_8.h5
corrupt_stab_msg.h5
cve_2020_10812.h5
deflate.h5
family_v16-000000.h5
family_v16-000001.h5
Expand Down
18 changes: 9 additions & 9 deletions test/chunk_info.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1508,9 +1508,9 @@ test_chunk_info_version2_btrees(const char *filename, hid_t fapl)

typedef struct chunk_iter_info_t {
hsize_t offset[2];
uint32_t filter_mask;
unsigned filter_mask;
haddr_t addr;
uint32_t nbytes;
hsize_t size;
} chunk_iter_info_t;

typedef struct chunk_iter_udata_t {
Expand All @@ -1519,7 +1519,7 @@ typedef struct chunk_iter_udata_t {
} chunk_iter_udata_t;

static int
iter_cb(const hsize_t *offset, uint32_t filter_mask, haddr_t addr, uint32_t nbytes, void *op_data)
iter_cb(const hsize_t *offset, unsigned filter_mask, haddr_t addr, hsize_t size, void *op_data)
{
chunk_iter_udata_t *cidata = (chunk_iter_udata_t *)op_data;
int idx = cidata->last_index + 1;
Expand All @@ -1528,25 +1528,25 @@ iter_cb(const hsize_t *offset, uint32_t filter_mask, haddr_t addr, uint32_t nbyt
cidata->chunk_info[idx].offset[1] = offset[1];
cidata->chunk_info[idx].filter_mask = filter_mask;
cidata->chunk_info[idx].addr = addr;
cidata->chunk_info[idx].nbytes = nbytes;
cidata->chunk_info[idx].size = size;

cidata->last_index++;

return H5_ITER_CONT;
}

static int
iter_cb_stop(const hsize_t H5_ATTR_UNUSED *offset, uint32_t H5_ATTR_UNUSED filter_mask,
haddr_t H5_ATTR_UNUSED addr, uint32_t H5_ATTR_UNUSED nbytes, void *op_data)
iter_cb_stop(const hsize_t H5_ATTR_UNUSED *offset, unsigned H5_ATTR_UNUSED filter_mask,
haddr_t H5_ATTR_UNUSED addr, hsize_t H5_ATTR_UNUSED size, void *op_data)
{
chunk_iter_info_t **chunk_info = (chunk_iter_info_t **)op_data;
*chunk_info += 1;
return H5_ITER_STOP;
}

static int
iter_cb_fail(const hsize_t H5_ATTR_UNUSED *offset, uint32_t H5_ATTR_UNUSED filter_mask,
haddr_t H5_ATTR_UNUSED addr, uint32_t H5_ATTR_UNUSED nbytes, void *op_data)
iter_cb_fail(const hsize_t H5_ATTR_UNUSED *offset, unsigned H5_ATTR_UNUSED filter_mask,
haddr_t H5_ATTR_UNUSED addr, hsize_t H5_ATTR_UNUSED size, void *op_data)
{
chunk_iter_info_t **chunk_info = (chunk_iter_info_t **)op_data;
*chunk_info += 1;
Expand Down Expand Up @@ -1718,7 +1718,7 @@ test_basic_query(hid_t fapl)
FAIL_PUTS_ERROR("offset[1] mismatch");
if (chunk_infos[0].filter_mask != 0)
FAIL_PUTS_ERROR("filter mask mismatch");
if (chunk_infos[0].nbytes != 96)
if (chunk_infos[0].size != 96)
FAIL_PUTS_ERROR("size mismatch");

if (chunk_infos[1].offset[0] != CHUNK_NX)
Expand Down
Binary file added test/cve_2020_10812.h5
View file
Binary file not shown.
Loading

0 comments on commit 83c7f92

Please sign in to comment.