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.

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

Prevent stack overflows in H5E__push_stack #4264

Merged
merged 1 commit into from
Mar 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 3 additions & 3 deletions src/H5Eint.c
Original file line number Diff line number Diff line change
Expand Up @@ -730,13 +730,13 @@ H5E__push_stack(H5E_t *estack, const char *file, const char *func, unsigned line

if (estack->nused < H5E_NSLOTS) {
/* Increment the IDs to indicate that they are used in this stack */
if (H5I_inc_ref(cls_id, false) < 0)
if (H5I_inc_ref_noherr(cls_id, false) < 0)
HGOTO_DONE(FAIL);
estack->slot[estack->nused].cls_id = cls_id;
if (H5I_inc_ref(maj_id, false) < 0)
if (H5I_inc_ref_noherr(maj_id, false) < 0)
HGOTO_DONE(FAIL);
estack->slot[estack->nused].maj_num = maj_id;
if (H5I_inc_ref(min_id, false) < 0)
if (H5I_inc_ref_noherr(min_id, false) < 0)
HGOTO_DONE(FAIL);
estack->slot[estack->nused].min_num = min_id;
/* The 'func' & 'file' strings are statically allocated (by the compiler)
Expand Down
74 changes: 68 additions & 6 deletions src/H5Iint.c
Original file line number Diff line number Diff line change
Expand Up @@ -1230,6 +1230,27 @@ H5I_dec_app_ref_always_close_async(hid_t id, void **token)
FUNC_LEAVE_NOAPI(ret_value)
} /* end H5I_dec_app_ref_always_close_async() */

/*-------------------------------------------------------------------------
* Function: H5I_do_inc_ref
*
* Purpose: Helper function for H5I_inc_ref/H5I_inc_ref_noherr to
* actually increment the reference count for an object.
*
* Return: The new reference count (can't fail)
*
*-------------------------------------------------------------------------
*/
static inline int
H5I_do_inc_ref(H5I_id_info_t *info, bool app_ref)
{
/* Adjust reference counts */
++(info->count);
if (app_ref)
++(info->app_count);

return (int)(app_ref ? info->app_count : info->count);
}

/*-------------------------------------------------------------------------
* Function: H5I_inc_ref
*
Expand All @@ -1255,18 +1276,59 @@ H5I_inc_ref(hid_t id, bool app_ref)
if (NULL == (info = H5I__find_id(id)))
HGOTO_ERROR(H5E_ID, H5E_BADID, (-1), "can't locate ID");

/* Adjust reference counts */
++(info->count);
if (app_ref)
++(info->app_count);

/* Set return value */
ret_value = (int)(app_ref ? info->app_count : info->count);
ret_value = H5I_do_inc_ref(info, app_ref);

done:
FUNC_LEAVE_NOAPI(ret_value)
} /* end H5I_inc_ref() */

/*-------------------------------------------------------------------------
* Function: H5I_inc_ref_noherr
*
* Purpose: Increment the reference count for an object. Exactly like
* H5I_inc_ref, except that it makes use of HGOTO_DONE on
* failure instead of HGOTO_ERROR. This function is
* specifically meant to be used in the H5E package, where we
* have to avoid calling any function or macro that may call
* HGOTO_ERROR and similar. Otherwise, we can cause a stack
* overflow that looks like (for example):
*
* H5E_printf_stack()
* H5E__push_stack()
* H5I_inc_ref()
* H5I__find_id() (FAIL)
* HGOTO_ERROR()
* H5E_printf_stack()
* ...
*
* Return: Success: The new reference count
* Failure: -1
*
*-------------------------------------------------------------------------
*/
int
H5I_inc_ref_noherr(hid_t id, bool app_ref)
{
H5I_id_info_t *info = NULL; /* Pointer to the ID info */
int ret_value = 0; /* Return value */

FUNC_ENTER_NOAPI_NOERR

/* Sanity check */
assert(id >= 0);

/* General lookup of the ID */
if (NULL == (info = H5I__find_id(id)))
HGOTO_DONE((-1));

/* Set return value */
ret_value = H5I_do_inc_ref(info, app_ref);

done:
FUNC_LEAVE_NOAPI(ret_value)
} /* end H5I_inc_ref_noherr() */

/*-------------------------------------------------------------------------
* Function: H5I_get_ref
*
Expand Down
1 change: 1 addition & 0 deletions src/H5Iprivate.h
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ H5_DLL H5I_type_t H5I_get_type(hid_t id);
H5_DLL herr_t H5I_iterate(H5I_type_t type, H5I_search_func_t func, void *udata, bool app_ref);
H5_DLL int H5I_get_ref(hid_t id, bool app_ref);
H5_DLL int H5I_inc_ref(hid_t id, bool app_ref);
H5_DLL int H5I_inc_ref_noherr(hid_t id, bool app_ref);
H5_DLL int H5I_dec_ref(hid_t id);
H5_DLL int H5I_dec_app_ref(hid_t id);
H5_DLL int H5I_dec_app_ref_async(hid_t id, void **token);
Expand Down
Loading