Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix segfault when closing datatype during failure in H5Topen2 #4683

Merged
merged 1 commit into from
Aug 2, 2024
Merged

Fix segfault when closing datatype during failure in H5Topen2 #4683

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 37 additions & 1 deletion src/H5Tcommit.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ static herr_t H5T__commit_api_common(hid_t loc_id, const char *name, hid_t type_
static hid_t H5T__open_api_common(hid_t loc_id, const char *name, hid_t tapl_id, void **token_ptr,
H5VL_object_t **_vol_obj_ptr);
static H5T_t *H5T__open_oid(const H5G_loc_t *loc);
static herr_t H5T_destruct_datatype(void *datatype, H5VL_t *vol_connector);

/*********************/
/* Public Variables */
Expand Down Expand Up @@ -662,7 +663,7 @@ H5T__open_api_common(hid_t loc_id, const char *name, hid_t tapl_id, void **token
done:
/* Cleanup on error */
if (H5I_INVALID_HID == ret_value)
if (dt && H5VL_datatype_close(*vol_obj_ptr, H5P_DATASET_XFER_DEFAULT, H5_REQUEST_NULL) < 0)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This VOL object pointer actually points to the VOL object that was passed in to H5Topen2 for loc_id rather than pointing to the VOL object for the committed datatype. When H5VL_register above fails, dt is left as a typeless pointer not attached to a VOL object, so we have to wrap it in one before being able to call H5VL_datatype_close on it.

if (dt && H5T_destruct_datatype(dt, (*vol_obj_ptr)->connector) < 0)
HDONE_ERROR(H5E_DATATYPE, H5E_CLOSEERROR, H5I_INVALID_HID, "unable to release datatype");

FUNC_LEAVE_NOAPI(ret_value)
Expand Down Expand Up @@ -1260,6 +1261,41 @@ H5T_construct_datatype(H5VL_object_t *vol_obj)
FUNC_LEAVE_NOAPI(ret_value)
} /* end H5T_construct_datatype() */

/*-------------------------------------------------------------------------
* Function: H5T_destruct_datatype
*
* Purpose: Helper function to free a committed datatype object that
* hasn't yet been wrapped within a VOL object. This usually
* happens when a failure occurs during opening a committed
* datatype. When this happens, the datatype must be wrapped
* inside a temporary VOL object in order to route the close
* operation through the stack of VOL connectors.
*
* Return: Non-negative on success/Negative on failure
*
*-------------------------------------------------------------------------
*/
static herr_t
H5T_destruct_datatype(void *datatype, H5VL_t *vol_connector)
{
H5VL_object_t *vol_obj = NULL;
herr_t ret_value = FAIL;

FUNC_ENTER_NOAPI(FAIL)

if (NULL == (vol_obj = H5VL_create_object(datatype, vol_connector)))
HGOTO_ERROR(H5E_DATATYPE, H5E_CANTALLOC, FAIL, "can't create VOL object for committed datatype");

if (H5VL_datatype_close(vol_obj, H5P_DATASET_XFER_DEFAULT, H5_REQUEST_NULL) < 0)
HGOTO_ERROR(H5E_DATATYPE, H5E_CLOSEERROR, FAIL, "unable to release datatype");

done:
if (vol_obj && H5VL_free_object(vol_obj) < 0)
HDONE_ERROR(H5E_DATATYPE, H5E_CANTFREE, FAIL, "can't free VOL object");

FUNC_LEAVE_NOAPI(ret_value)
} /* end H5T_destruct_datatype() */

/*-------------------------------------------------------------------------
* Function: H5T_get_named_type
*
Expand Down
8 changes: 7 additions & 1 deletion src/H5VLint.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -578,6 +578,12 @@ H5VL__new_vol_obj(H5I_type_t type, void *object, H5VL_t *vol_connector, bool wra
if (NULL == ret_value) {
if (conn_rc_incr && H5VL_conn_dec_rc(vol_connector) < 0)
HDONE_ERROR(H5E_VOL, H5E_CANTDEC, NULL, "unable to decrement ref count on VOL connector");

if (new_vol_obj) {
if (wrap_obj && new_vol_obj->data)
(void)H5VL_object_unwrap(new_vol_obj);
(void)H5FL_FREE(H5VL_object_t, new_vol_obj);
}
} /* end if */

FUNC_LEAVE_NOAPI(ret_value)
Expand Down Expand Up @@ -698,7 +704,7 @@ H5VL_register(H5I_type_t type, void *object, H5VL_t *vol_connector, bool app_ref
/* Set up VOL object for the passed-in data */
/* (Does not wrap object, since it's from a VOL callback) */
if (NULL == (vol_obj = H5VL__new_vol_obj(type, object, vol_connector, false)))
HGOTO_ERROR(H5E_VOL, H5E_CANTCREATE, FAIL, "can't create VOL object");
HGOTO_ERROR(H5E_VOL, H5E_CANTCREATE, H5I_INVALID_HID, "can't create VOL object");

/* Register VOL object as _object_ type, for future object API calls */
if ((ret_value = H5I_register(type, vol_obj, app_ref)) < 0)
Expand Down
Loading