mirror of
https://github.com/HDFGroup/hdf5.git
synced 2025-01-30 15:32:37 +08:00
Fix an issue where compound datatype member IDs can be leaked during conversion (#4459)
Also fixes issues with handling of partially initialized datatypes during library shutdown
This commit is contained in:
jhendersonHDF
GitHub
parent
f17ca5689e
commit
ac7b5ce314
@ -749,6 +749,22 @@ Bug Fixes since HDF5-1.14.0 release
|
||||
===================================
|
||||
Library
|
||||
-------
|
||||
- Fixed a leak of datatype IDs created internally during datatype conversion
|
||||
|
||||
Fixed an issue where the library could leak IDs that it creates internally
|
||||
for compound datatype members during datatype conversion. When the library's
|
||||
table of datatype conversion functions is modified (such as when a new
|
||||
conversion function is registered with the library from within an application),
|
||||
the compound datatype conversion function has to recalculate data that it
|
||||
has cached. When recalculating that data, the library was registering new
|
||||
IDs for each of the members of the source and destination compound datatypes
|
||||
involved in the conversion process and was overwriting the old cached IDs
|
||||
without first closing them. This would result in use-after-free issues due
|
||||
to multiple IDs pointing to the same internal H5T_t structure, as well as
|
||||
crashes due to the library not gracefully handling partially initialized or
|
||||
partially freed datatypes on library termination.
|
||||
|
||||
Fixes h5py GitHub #2419
|
||||
|
||||
- Fixed function H5Requal actually to compare the reference pointers
|
||||
|
||||
|
||||
13
src/H5T.c
13
src/H5T.c
@ -1655,12 +1655,11 @@ H5T__unlock_cb(void *_dt, hid_t H5_ATTR_UNUSED id, void *_udata)
|
||||
FUNC_ENTER_PACKAGE_NOERR
|
||||
|
||||
assert(dt);
|
||||
assert(dt->shared);
|
||||
|
||||
if (H5T_STATE_IMMUTABLE == dt->shared->state) {
|
||||
if (dt->shared && (H5T_STATE_IMMUTABLE == dt->shared->state)) {
|
||||
dt->shared->state = H5T_STATE_RDONLY;
|
||||
(*n)++;
|
||||
} /* end if */
|
||||
}
|
||||
|
||||
FUNC_LEAVE_NOAPI(SUCCEED)
|
||||
} /* end H5T__unlock_cb() */
|
||||
@ -1874,7 +1873,6 @@ H5T__close_cb(H5T_t *dt, void **request)
|
||||
|
||||
/* Sanity check */
|
||||
assert(dt);
|
||||
assert(dt->shared);
|
||||
|
||||
/* If this datatype is VOL-managed (i.e.: has a VOL object),
|
||||
* close it through the VOL connector.
|
||||
@ -4154,10 +4152,10 @@ H5T_close_real(H5T_t *dt)
|
||||
FUNC_ENTER_NOAPI(FAIL)
|
||||
|
||||
/* Sanity check */
|
||||
assert(dt && dt->shared);
|
||||
assert(dt);
|
||||
|
||||
/* Clean up resources, depending on shared state */
|
||||
if (dt->shared->state != H5T_STATE_OPEN) {
|
||||
if (dt->shared && (dt->shared->state != H5T_STATE_OPEN)) {
|
||||
if (H5T__free(dt) < 0)
|
||||
HGOTO_ERROR(H5E_DATATYPE, H5E_CANTFREE, FAIL, "unable to free datatype");
|
||||
|
||||
@ -4194,10 +4192,9 @@ H5T_close(H5T_t *dt)
|
||||
|
||||
/* Sanity check */
|
||||
assert(dt);
|
||||
assert(dt->shared);
|
||||
|
||||
/* Named datatype cleanups */
|
||||
if (dt->shared->state == H5T_STATE_OPEN) {
|
||||
if (dt->shared && (dt->shared->state == H5T_STATE_OPEN)) {
|
||||
/* Decrement refcount count on open named datatype */
|
||||
dt->shared->fo_count--;
|
||||
|
||||
|
||||
@ -264,21 +264,32 @@ H5T__conv_struct_init(const H5T_t *src, const H5T_t *dst, H5T_cdata_t *cdata, co
|
||||
if (need_ids) {
|
||||
hid_t tid;
|
||||
|
||||
if ((tid = H5I_register(H5I_DATATYPE, priv->src_memb[i], false)) < 0) {
|
||||
H5T__conv_struct_free(priv);
|
||||
cdata->priv = NULL;
|
||||
HGOTO_ERROR(H5E_DATATYPE, H5E_CANTREGISTER, FAIL,
|
||||
"can't register ID for source compound member datatype");
|
||||
/* Only register new IDs for the source and destination member datatypes
|
||||
* if IDs weren't already registered for them. If the cached conversion
|
||||
* information has to be recalculated (in the case where the library's
|
||||
* table of conversion functions is modified), the same IDs can be reused
|
||||
* since the only information that needs to be recalculated is the conversion
|
||||
* paths used.
|
||||
*/
|
||||
if (priv->src_memb_id[i] == H5I_INVALID_HID) {
|
||||
if ((tid = H5I_register(H5I_DATATYPE, priv->src_memb[i], false)) < 0) {
|
||||
H5T__conv_struct_free(priv);
|
||||
cdata->priv = NULL;
|
||||
HGOTO_ERROR(H5E_DATATYPE, H5E_CANTREGISTER, FAIL,
|
||||
"can't register ID for source compound member datatype");
|
||||
}
|
||||
priv->src_memb_id[i] = tid;
|
||||
}
|
||||
priv->src_memb_id[i] = tid;
|
||||
|
||||
if ((tid = H5I_register(H5I_DATATYPE, priv->dst_memb[src2dst[i]], false)) < 0) {
|
||||
H5T__conv_struct_free(priv);
|
||||
cdata->priv = NULL;
|
||||
HGOTO_ERROR(H5E_DATATYPE, H5E_CANTREGISTER, FAIL,
|
||||
"can't register ID for destination compound member datatype");
|
||||
if (priv->dst_memb_id[src2dst[i]] == H5I_INVALID_HID) {
|
||||
if ((tid = H5I_register(H5I_DATATYPE, priv->dst_memb[src2dst[i]], false)) < 0) {
|
||||
H5T__conv_struct_free(priv);
|
||||
cdata->priv = NULL;
|
||||
HGOTO_ERROR(H5E_DATATYPE, H5E_CANTREGISTER, FAIL,
|
||||
"can't register ID for destination compound member datatype");
|
||||
}
|
||||
priv->dst_memb_id[src2dst[i]] = tid;
|
||||
}
|
||||
priv->dst_memb_id[src2dst[i]] = tid;
|
||||
}
|
||||
} /* end if */
|
||||
} /* end for */
|
||||
|
||||
209
test/dtypes.c
209
test/dtypes.c
@ -3913,6 +3913,214 @@ error:
|
||||
return 1;
|
||||
} /* end test_user_compound_conversion() */
|
||||
|
||||
/*-------------------------------------------------------------------------
|
||||
* Function: test_compound_member_convert_id_leak_func1
|
||||
*
|
||||
* Purpose: Datatype conversion function for the
|
||||
* test_compound_member_convert_id_leak test that just
|
||||
* converts a float value to a double value with a cast.
|
||||
*
|
||||
* Return: Success: 0
|
||||
* Failure: number of errors
|
||||
*
|
||||
*-------------------------------------------------------------------------
|
||||
*/
|
||||
static herr_t
|
||||
test_compound_member_convert_id_leak_func1(hid_t src_id, hid_t dst_id, H5T_cdata_t *cdata, size_t nelmts,
|
||||
size_t H5_ATTR_UNUSED buf_stride, size_t H5_ATTR_UNUSED bkg_stride,
|
||||
void *buf, void H5_ATTR_UNUSED *bkg,
|
||||
hid_t H5_ATTR_UNUSED dset_xfer_plist)
|
||||
{
|
||||
float tmp_val;
|
||||
double tmp_val2;
|
||||
|
||||
switch (cdata->command) {
|
||||
case H5T_CONV_INIT:
|
||||
if (!H5Tequal(src_id, H5T_NATIVE_FLOAT) || !H5Tequal(dst_id, H5T_NATIVE_DOUBLE))
|
||||
return FAIL;
|
||||
break;
|
||||
case H5T_CONV_CONV:
|
||||
if (nelmts != 1)
|
||||
return FAIL;
|
||||
|
||||
memcpy(&tmp_val, buf, sizeof(float));
|
||||
tmp_val2 = (double)tmp_val;
|
||||
memcpy(buf, &tmp_val2, sizeof(double));
|
||||
|
||||
break;
|
||||
case H5T_CONV_FREE:
|
||||
break;
|
||||
default:
|
||||
return FAIL;
|
||||
}
|
||||
|
||||
return SUCCEED;
|
||||
}
|
||||
|
||||
/*-------------------------------------------------------------------------
|
||||
* Function: test_compound_member_convert_id_leak_func2
|
||||
*
|
||||
* Purpose: Datatype conversion function for the
|
||||
* test_compound_member_convert_id_leak test that just
|
||||
* returns the double value 0.1.
|
||||
*
|
||||
* Return: Success: 0
|
||||
* Failure: number of errors
|
||||
*
|
||||
*-------------------------------------------------------------------------
|
||||
*/
|
||||
static herr_t
|
||||
test_compound_member_convert_id_leak_func2(hid_t src_id, hid_t dst_id, H5T_cdata_t *cdata, size_t nelmts,
|
||||
size_t H5_ATTR_UNUSED buf_stride, size_t H5_ATTR_UNUSED bkg_stride,
|
||||
void *buf, void H5_ATTR_UNUSED *bkg,
|
||||
hid_t H5_ATTR_UNUSED dset_xfer_plist)
|
||||
{
|
||||
double tmp_val = 0.1;
|
||||
|
||||
switch (cdata->command) {
|
||||
case H5T_CONV_INIT:
|
||||
if (!H5Tequal(src_id, H5T_NATIVE_FLOAT) || !H5Tequal(dst_id, H5T_NATIVE_DOUBLE))
|
||||
return FAIL;
|
||||
break;
|
||||
case H5T_CONV_CONV:
|
||||
if (nelmts != 1)
|
||||
return FAIL;
|
||||
|
||||
memcpy(buf, &tmp_val, sizeof(double));
|
||||
|
||||
break;
|
||||
case H5T_CONV_FREE:
|
||||
break;
|
||||
default:
|
||||
return FAIL;
|
||||
}
|
||||
|
||||
return SUCCEED;
|
||||
}
|
||||
|
||||
/*-------------------------------------------------------------------------
|
||||
* Function: test_compound_member_convert_id_leak
|
||||
*
|
||||
* Purpose: Tests for an issue where IDs that are registered for
|
||||
* compound datatype members during datatype conversion were
|
||||
* leaked when the library's conversion path table is modified
|
||||
* and the compound conversion path recalculates its cached
|
||||
* data.
|
||||
*
|
||||
* Return: Success: 0
|
||||
* Failure: number of errors
|
||||
*
|
||||
*-------------------------------------------------------------------------
|
||||
*/
|
||||
static int
|
||||
test_compound_member_convert_id_leak(void)
|
||||
{
|
||||
int64_t num_dtype_ids = 0;
|
||||
float val1;
|
||||
double val2;
|
||||
double bkg;
|
||||
hid_t tid1 = H5I_INVALID_HID;
|
||||
hid_t tid2 = H5I_INVALID_HID;
|
||||
|
||||
TESTING("compound conversion member ID leak");
|
||||
|
||||
if ((tid1 = H5Tcreate(H5T_COMPOUND, sizeof(float))) < 0)
|
||||
TEST_ERROR;
|
||||
if ((tid2 = H5Tcreate(H5T_COMPOUND, sizeof(double))) < 0)
|
||||
TEST_ERROR;
|
||||
|
||||
if (H5Tinsert(tid1, "mem", 0, H5T_NATIVE_FLOAT) < 0)
|
||||
TEST_ERROR;
|
||||
if (H5Tinsert(tid2, "mem", 0, H5T_NATIVE_DOUBLE) < 0)
|
||||
TEST_ERROR;
|
||||
|
||||
/* Store the current number of datatype IDs registered */
|
||||
if ((num_dtype_ids = H5I_nmembers(H5I_DATATYPE)) < 0)
|
||||
TEST_ERROR;
|
||||
|
||||
/* Convert a value from float to double */
|
||||
val1 = 3.0f;
|
||||
val2 = 0.0;
|
||||
memcpy(&val2, &val1, sizeof(float));
|
||||
if (H5Tconvert(tid1, tid2, 1, &val2, &bkg, H5P_DEFAULT) < 0)
|
||||
TEST_ERROR;
|
||||
|
||||
/* Make sure the number of datatype IDs registered didn't change */
|
||||
if (num_dtype_ids != H5I_nmembers(H5I_DATATYPE))
|
||||
TEST_ERROR;
|
||||
|
||||
/* Register a custom conversion function from float to double
|
||||
* and convert the value again
|
||||
*/
|
||||
if (H5Tregister(H5T_PERS_HARD, "myflttodbl", H5T_NATIVE_FLOAT, H5T_NATIVE_DOUBLE,
|
||||
test_compound_member_convert_id_leak_func1) < 0)
|
||||
TEST_ERROR;
|
||||
|
||||
val1 = 3.0f;
|
||||
val2 = 0.0;
|
||||
memcpy(&val2, &val1, sizeof(float));
|
||||
if (H5Tconvert(tid1, tid2, 1, &val2, &bkg, H5P_DEFAULT) < 0)
|
||||
TEST_ERROR;
|
||||
|
||||
/* Since an application conversion function was used, two IDs should
|
||||
* have been registered, one for the source type and one for the
|
||||
* destination type
|
||||
*/
|
||||
num_dtype_ids += 2;
|
||||
|
||||
/* Make sure the number of datatype IDs registered is correct */
|
||||
if (num_dtype_ids != H5I_nmembers(H5I_DATATYPE))
|
||||
TEST_ERROR;
|
||||
|
||||
if (H5Tunregister(H5T_PERS_HARD, "myflttodbl", H5T_NATIVE_FLOAT, H5T_NATIVE_DOUBLE,
|
||||
test_compound_member_convert_id_leak_func1) < 0)
|
||||
TEST_ERROR;
|
||||
|
||||
/* Register a different custom conversion function from float to double
|
||||
* and convert the value again
|
||||
*/
|
||||
if (H5Tregister(H5T_PERS_HARD, "myflttodbl", H5T_NATIVE_FLOAT, H5T_NATIVE_DOUBLE,
|
||||
test_compound_member_convert_id_leak_func2) < 0)
|
||||
TEST_ERROR;
|
||||
|
||||
val1 = 3.0f;
|
||||
val2 = 0.0;
|
||||
memcpy(&val2, &val1, sizeof(float));
|
||||
if (H5Tconvert(tid1, tid2, 1, &val2, &bkg, H5P_DEFAULT) < 0)
|
||||
TEST_ERROR;
|
||||
|
||||
/* Make sure the number of datatype IDs registered didn't change */
|
||||
if (num_dtype_ids != H5I_nmembers(H5I_DATATYPE))
|
||||
TEST_ERROR;
|
||||
|
||||
if (H5Tunregister(H5T_PERS_HARD, "myflttodbl", H5T_NATIVE_FLOAT, H5T_NATIVE_DOUBLE,
|
||||
test_compound_member_convert_id_leak_func2) < 0)
|
||||
TEST_ERROR;
|
||||
|
||||
if (H5Tclose(tid1) < 0)
|
||||
TEST_ERROR;
|
||||
if (H5Tclose(tid2) < 0)
|
||||
TEST_ERROR;
|
||||
|
||||
PASSED();
|
||||
|
||||
return 0;
|
||||
|
||||
error:
|
||||
H5E_BEGIN_TRY
|
||||
{
|
||||
H5Tunregister(H5T_PERS_HARD, "myflttodbl", H5T_NATIVE_FLOAT, H5T_NATIVE_DOUBLE,
|
||||
test_compound_member_convert_id_leak_func1);
|
||||
H5Tunregister(H5T_PERS_HARD, "myflttodbl", H5T_NATIVE_FLOAT, H5T_NATIVE_DOUBLE,
|
||||
test_compound_member_convert_id_leak_func2);
|
||||
H5Tclose(tid1);
|
||||
H5Tclose(tid2);
|
||||
}
|
||||
H5E_END_TRY;
|
||||
|
||||
return 1;
|
||||
} /* end test_compound_member_convert_id_leak() */
|
||||
|
||||
/*-------------------------------------------------------------------------
|
||||
* Function: test_query
|
||||
*
|
||||
@ -10198,6 +10406,7 @@ main(void)
|
||||
nerrors += test_compound_17();
|
||||
nerrors += test_compound_18();
|
||||
nerrors += test_user_compound_conversion();
|
||||
nerrors += test_compound_member_convert_id_leak();
|
||||
nerrors += test_conv_enum_1();
|
||||
nerrors += test_conv_enum_2();
|
||||
nerrors += test_conv_bitfield();
|
||||
|
||||
Loading…
Reference in New Issue
Block a user