Skip to content

Fix several undefined behavior issues noted by UBsan #6319

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

Open
jhendersonHDF wants to merge 2 commits into
base: develop
Choose a base branch
from
+241 −188
Open

Fix several undefined behavior issues noted by UBsan #6319

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
77910b0
Fix several undefined behavior issues noted by UBsan
jhendersonHDF Mar 25, 2026
477e570
Fix error message major error code
jhendersonHDF Mar 26, 2026
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
1 change: 1 addition & 0 deletions .github/workflows/analysis.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -414,6 +414,7 @@ jobs:
set (ADD_BUILD_OPTIONS "${ADD_BUILD_OPTIONS} -DHDF5_BUILD_JAVA:BOOL=OFF")
set (ADD_BUILD_OPTIONS "${ADD_BUILD_OPTIONS} -DHDF5_BUILD_CPP_LIB:BOOL=ON")
set (ADD_BUILD_OPTIONS "${ADD_BUILD_OPTIONS} -DHDF5_BUILD_FORTRAN:BOOL=OFF")
set (ADD_BUILD_OPTIONS "${ADD_BUILD_OPTIONS} -DHDF5_ENABLE_NONSTANDARD_FEATURE_FLOAT16:BOOL=OFF")
Copy link
Collaborator Author

@jhendersonHDF jhendersonHDF Mar 25, 2026

Choose a reason for hiding this comment

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

The _Float16 type currently crashes UBSan, preventing further analysis in dt_arith.c.

set (ADD_BUILD_OPTIONS "${ADD_BUILD_OPTIONS} -DHDF5_ENABLE_SANITIZERS:BOOL=ON")
set (ADD_BUILD_OPTIONS "${ADD_BUILD_OPTIONS} -DHDF5_USE_SANITIZER:STRING=Undefined")
set (ADD_BUILD_OPTIONS "${ADD_BUILD_OPTIONS} -DHDF5_ENABLE_ZLIB_SUPPORT:BOOL=OFF")
Expand Down
21 changes: 11 additions & 10 deletions src/H5Aint.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ typedef struct {
/* Local Prototypes */
/********************/

static herr_t H5A__close_cb(H5VL_object_t *attr_vol_obj, void **request);
static herr_t H5A__close_cb(void *attr_vol_obj, void **request);
Copy link
Collaborator Author

@jhendersonHDF jhendersonHDF Mar 25, 2026

Choose a reason for hiding this comment

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

There were several instances in the library where an H5I_free_t callback was declared with some type parameter instead of void * and then casted to H5I_free_t, causing UBSan to complain. The typing is lost for these callbacks, but several other H5I_free_t callbacks in the library already use the correct void * parameter anyway.

static herr_t H5A__compact_build_table_cb(H5O_t *oh, H5O_mesg_t *mesg /*in,out*/, unsigned sequence,
void *_udata /*in,out*/);
static herr_t H5A__dense_build_table_cb(const H5A_t *attr, void *_udata);
Expand Down Expand Up @@ -124,10 +124,10 @@ H5FL_SEQ_DEFINE_STATIC(H5A_t_ptr);

/* Attribute ID class */
static const H5I_class_t H5I_ATTR_CLS[1] = {{
H5I_ATTR, /* ID class value */
0, /* Class flags */
0, /* # of reserved IDs for class */
(H5I_free_t)H5A__close_cb /* Callback routine for closing objects of this class */
H5I_ATTR, /* ID class value */
0, /* Class flags */
0, /* # of reserved IDs for class */
H5A__close_cb /* Callback routine for closing objects of this class */
}};

/* Flag indicating "top" of interface has been initialized */
Expand Down Expand Up @@ -1277,21 +1277,22 @@ H5A__shared_free(H5A_t *attr)
*-------------------------------------------------------------------------
*/
static herr_t
H5A__close_cb(H5VL_object_t *attr_vol_obj, void **request)
H5A__close_cb(void *attr_vol_obj, void **request)
{
herr_t ret_value = SUCCEED; /* Return value */
H5VL_object_t *attr_vol_obj_p = (H5VL_object_t *)attr_vol_obj;
herr_t ret_value = SUCCEED; /* Return value */

FUNC_ENTER_PACKAGE

/* Sanity check */
assert(attr_vol_obj);
assert(attr_vol_obj_p);

/* Close the attribute */
if (H5VL_attr_close(attr_vol_obj, H5P_DATASET_XFER_DEFAULT, request) < 0)
if (H5VL_attr_close(attr_vol_obj_p, H5P_DATASET_XFER_DEFAULT, request) < 0)
HGOTO_ERROR(H5E_ATTR, H5E_CLOSEERROR, FAIL, "problem closing attribute");

/* Free the VOL object */
if (H5VL_free_object(attr_vol_obj) < 0)
if (H5VL_free_object(attr_vol_obj_p) < 0)
HGOTO_ERROR(H5E_ATTR, H5E_CANTDEC, FAIL, "unable to free VOL object");

done:
Expand Down
21 changes: 11 additions & 10 deletions src/H5Dint.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ static herr_t H5D__build_file_prefix(const H5D_t *dset, H5F_prefix_open_t prefix
static herr_t H5D__open_oid(H5D_t *dataset, hid_t dapl_id);
static herr_t H5D__init_storage(H5D_t *dset, bool full_overwrite, hsize_t old_dim[]);
static herr_t H5D__append_flush_setup(H5D_t *dset, hid_t dapl_id);
static herr_t H5D__close_cb(H5VL_object_t *dset_vol_obj, void **request);
static herr_t H5D__close_cb(void *dset_vol_obj, void **request);
static herr_t H5D__use_minimized_dset_headers(H5F_t *file, bool *minimize);
static herr_t H5D__prepare_minimized_oh(H5F_t *file, H5D_t *dset, H5O_loc_t *oloc);
static size_t H5D__calculate_minimum_header_size(H5F_t *file, H5D_t *dset, H5O_t *ohdr);
Expand Down Expand Up @@ -133,10 +133,10 @@ H5_WARN_LARGE_STACK_OBJECTS_ON

/* Dataset ID class */
static const H5I_class_t H5I_DATASET_CLS[1] = {{
H5I_DATASET, /* ID class value */
0, /* Class flags */
0, /* # of reserved IDs for class */
(H5I_free_t)H5D__close_cb /* Callback routine for closing objects of this class */
H5I_DATASET, /* ID class value */
0, /* Class flags */
0, /* # of reserved IDs for class */
H5D__close_cb /* Callback routine for closing objects of this class */
}};

/* Flag indicating "top" of interface has been initialized */
Expand Down Expand Up @@ -332,22 +332,23 @@ H5D_term_package(void)
*-------------------------------------------------------------------------
*/
static herr_t
H5D__close_cb(H5VL_object_t *dset_vol_obj, void **request)
H5D__close_cb(void *dset_vol_obj, void **request)
{
herr_t ret_value = SUCCEED; /* Return value */
H5VL_object_t *dset_vol_obj_p = (H5VL_object_t *)dset_vol_obj;
herr_t ret_value = SUCCEED; /* Return value */

FUNC_ENTER_PACKAGE

/* Sanity check */
assert(dset_vol_obj);
assert(dset_vol_obj_p);

/* Close the dataset */
if (H5VL_dataset_close(dset_vol_obj, H5P_DATASET_XFER_DEFAULT, request) < 0)
if (H5VL_dataset_close(dset_vol_obj_p, H5P_DATASET_XFER_DEFAULT, request) < 0)
HGOTO_ERROR(H5E_DATASET, H5E_CLOSEERROR, FAIL, "unable to close dataset");

done:
/* Free the VOL object */
if (H5VL_free_object(dset_vol_obj) < 0)
if (H5VL_free_object(dset_vol_obj_p) < 0)
HGOTO_ERROR(H5E_DATASET, H5E_CANTDEC, FAIL, "unable to free VOL object");

FUNC_LEAVE_NOAPI(ret_value)
Expand Down
21 changes: 15 additions & 6 deletions src/H5Dscatgath.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1264,12 +1264,21 @@ H5D__scatgath_write_select(H5D_io_info_t *io_info)

if ((H5T_BKG_YES == dset_info->type_info.need_bkg) &&
!H5D__SCATGATH_USE_CMPD_OPT_WRITE(dset_info, io_info->sel_pieces[i]->in_place_tconv)) {
/* Non-const write_buf[i]. Use pointer math here to avoid const warnings. When
* there's a background buffer write_buf[i] always points inside the non-const tconv
* buf so this is OK. */
void *tmp_write_buf =
(void *)((uint8_t *)io_info->tconv_buf +
((const uint8_t *)write_bufs[i] - (const uint8_t *)io_info->tconv_buf));
void *tmp_write_buf;

if (io_info->sel_pieces[i]->in_place_tconv) {
Copy link
Collaborator Author

@jhendersonHDF jhendersonHDF Mar 25, 2026

Choose a reason for hiding this comment

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

@fortnern do these changes look correct? UBSan was complaining because in this particular case (where modifying of write buffers is enabled), the type conversion buffer wasn't allocated, so the previous logic was adding a non-zero offset to a NULL pointer. I'm not entirely certain how the testing passes currently given that, but it was definitely wrong. In the in_place_tconv case, it seemed like the logic above in the first loop already set each write_bufs[i] to point to the correct location in the write buffer. In fact, it also looks like each write_bufs[i] gets set to the correct place in io_info->tconv_buf above as well, so both cases might be able to be simplified to just the two lines below without needing all the casting in the in_place_tconv == false case.

H5_flexible_const_ptr_t flex_buf = {.cvp = write_bufs[i]};

tmp_write_buf = flex_buf.vp;
}
else {
/* Non-const write_buf[i]. Use pointer math here to avoid const warnings. When
* there's a background buffer write_buf[i] always points inside the non-const tconv
* buf so this is OK. */
tmp_write_buf =
(void *)((uint8_t *)io_info->tconv_buf +
((const uint8_t *)write_bufs[i] - (const uint8_t *)io_info->tconv_buf));
}

/* Do the data transform before the type conversion (since
* transforms must be done in the memory type). */
Expand Down
12 changes: 6 additions & 6 deletions src/H5EA.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -340,7 +340,7 @@ H5EA__lookup_elmt(const H5EA_t *ea, hsize_t idx, bool will_extend, unsigned thin
*thing = NULL;
*thing_elmt_buf = NULL;
*thing_elmt_idx = 0;
*thing_unprot_func = (H5EA__unprotect_func_t)NULL;
*thing_unprot_func = NULL;

/* Check if we should create the index block */
if (!H5_addr_defined(hdr->idx_blk_addr)) {
Expand Down Expand Up @@ -368,7 +368,7 @@ H5EA__lookup_elmt(const H5EA_t *ea, hsize_t idx, bool will_extend, unsigned thin
*thing = iblock;
*thing_elmt_buf = (uint8_t *)iblock->elmts;
*thing_elmt_idx = idx;
*thing_unprot_func = (H5EA__unprotect_func_t)H5EA__iblock_unprotect;
Copy link
Collaborator Author

@jhendersonHDF jhendersonHDF Mar 25, 2026

Choose a reason for hiding this comment

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

Similar to H5I_free_t, these callbacks were declaring in an incompatible way with H5EA__unprotect_func_t and then casted into it, causing UBSan to complain.

*thing_unprot_func = H5EA__iblock_unprotect;
} /* end if */
else {
unsigned sblk_idx; /* Which superblock does this index fall in? */
Expand Down Expand Up @@ -436,7 +436,7 @@ H5EA__lookup_elmt(const H5EA_t *ea, hsize_t idx, bool will_extend, unsigned thin
*thing = dblock;
*thing_elmt_buf = (uint8_t *)dblock->elmts;
*thing_elmt_idx = elmt_idx;
*thing_unprot_func = (H5EA__unprotect_func_t)H5EA__dblock_unprotect;
*thing_unprot_func = H5EA__dblock_unprotect;
} /* end if */
else {
size_t sblk_off; /* Offset of super block in index block array of super blocks */
Expand Down Expand Up @@ -569,7 +569,7 @@ H5EA__lookup_elmt(const H5EA_t *ea, hsize_t idx, bool will_extend, unsigned thin
*thing = dblk_page;
*thing_elmt_buf = (uint8_t *)dblk_page->elmts;
*thing_elmt_idx = elmt_idx;
*thing_unprot_func = (H5EA__unprotect_func_t)H5EA__dblk_page_unprotect;
*thing_unprot_func = H5EA__dblk_page_unprotect;
} /* end if */
else {
/* Protect data block */
Expand All @@ -593,7 +593,7 @@ H5EA__lookup_elmt(const H5EA_t *ea, hsize_t idx, bool will_extend, unsigned thin
*thing = dblock;
*thing_elmt_buf = (uint8_t *)dblock->elmts;
*thing_elmt_idx = elmt_idx;
*thing_unprot_func = (H5EA__unprotect_func_t)H5EA__dblock_unprotect;
*thing_unprot_func = H5EA__dblock_unprotect;
} /* end else */
} /* end else */
} /* end else */
Expand All @@ -608,7 +608,7 @@ H5EA__lookup_elmt(const H5EA_t *ea, hsize_t idx, bool will_extend, unsigned thin
*thing = NULL;
*thing_elmt_buf = NULL;
*thing_elmt_idx = 0;
*thing_unprot_func = (H5EA__unprotect_func_t)NULL;
*thing_unprot_func = NULL;
} /* end if */

/* Check for updating array statistics */
Expand Down
5 changes: 3 additions & 2 deletions src/H5EAdblkpage.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -257,9 +257,10 @@ H5EA__dblk_page_protect(H5EA_hdr_t *hdr, H5EA_sblock_t *parent, haddr_t dblk_pag
*-------------------------------------------------------------------------
*/
herr_t
H5EA__dblk_page_unprotect(H5EA_dblk_page_t *dblk_page, unsigned cache_flags)
H5EA__dblk_page_unprotect(void *_dblk_page, unsigned cache_flags)
{
herr_t ret_value = SUCCEED;
H5EA_dblk_page_t *dblk_page = (H5EA_dblk_page_t *)_dblk_page;
herr_t ret_value = SUCCEED;

FUNC_ENTER_PACKAGE

Expand Down
5 changes: 3 additions & 2 deletions src/H5EAdblock.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -336,9 +336,10 @@ H5EA__dblock_protect(H5EA_hdr_t *hdr, void *parent, haddr_t dblk_addr, size_t db
*-------------------------------------------------------------------------
*/
herr_t
H5EA__dblock_unprotect(H5EA_dblock_t *dblock, unsigned cache_flags)
H5EA__dblock_unprotect(void *_dblock, unsigned cache_flags)
{
herr_t ret_value = SUCCEED;
H5EA_dblock_t *dblock = (H5EA_dblock_t *)_dblock;
herr_t ret_value = SUCCEED;

FUNC_ENTER_PACKAGE

Expand Down
5 changes: 3 additions & 2 deletions src/H5EAiblock.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -324,9 +324,10 @@ H5EA__iblock_protect(H5EA_hdr_t *hdr, unsigned flags)
*-------------------------------------------------------------------------
*/
herr_t
H5EA__iblock_unprotect(H5EA_iblock_t *iblock, unsigned cache_flags)
H5EA__iblock_unprotect(void *_iblock, unsigned cache_flags)
{
herr_t ret_value = SUCCEED;
H5EA_iblock_t *iblock = (H5EA_iblock_t *)_iblock;
herr_t ret_value = SUCCEED;

FUNC_ENTER_PACKAGE

Expand Down
6 changes: 3 additions & 3 deletions src/H5EApkg.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -406,7 +406,7 @@ H5_DLL herr_t H5EA__hdr_dest(H5EA_hdr_t *hdr);
H5_DLL H5EA_iblock_t *H5EA__iblock_alloc(H5EA_hdr_t *hdr);
H5_DLL haddr_t H5EA__iblock_create(H5EA_hdr_t *hdr, bool *stats_changed);
H5_DLL H5EA_iblock_t *H5EA__iblock_protect(H5EA_hdr_t *hdr, unsigned flags);
H5_DLL herr_t H5EA__iblock_unprotect(H5EA_iblock_t *iblock, unsigned cache_flags);
H5_DLL herr_t H5EA__iblock_unprotect(void *_iblock, unsigned cache_flags);
H5_DLL herr_t H5EA__iblock_delete(H5EA_hdr_t *hdr);
H5_DLL herr_t H5EA__iblock_dest(H5EA_iblock_t *iblock);

Expand All @@ -428,7 +428,7 @@ H5_DLL haddr_t H5EA__dblock_create(H5EA_hdr_t *hdr, void *parent, bool *stats_c
H5_DLL unsigned H5EA__dblock_sblk_idx(const H5EA_hdr_t *hdr, hsize_t idx);
H5_DLL H5EA_dblock_t *H5EA__dblock_protect(H5EA_hdr_t *hdr, void *parent, haddr_t dblk_addr,
size_t dblk_nelmts, unsigned flags);
H5_DLL herr_t H5EA__dblock_unprotect(H5EA_dblock_t *dblock, unsigned cache_flags);
H5_DLL herr_t H5EA__dblock_unprotect(void *_dblock, unsigned cache_flags);
H5_DLL herr_t H5EA__dblock_delete(H5EA_hdr_t *hdr, void *parent, haddr_t dblk_addr, size_t dblk_nelmts);
H5_DLL herr_t H5EA__dblock_dest(H5EA_dblock_t *dblock);

Expand All @@ -437,7 +437,7 @@ H5_DLL H5EA_dblk_page_t *H5EA__dblk_page_alloc(H5EA_hdr_t *hdr, H5EA_sblock_t *p
H5_DLL herr_t H5EA__dblk_page_create(H5EA_hdr_t *hdr, H5EA_sblock_t *parent, haddr_t addr);
H5_DLL H5EA_dblk_page_t *H5EA__dblk_page_protect(H5EA_hdr_t *hdr, H5EA_sblock_t *parent,
haddr_t dblk_page_addr, unsigned flags);
H5_DLL herr_t H5EA__dblk_page_unprotect(H5EA_dblk_page_t *dblk_page, unsigned cache_flags);
H5_DLL herr_t H5EA__dblk_page_unprotect(void *_dblk_page, unsigned cache_flags);
H5_DLL herr_t H5EA__dblk_page_dest(H5EA_dblk_page_t *dblk_page);

/* Debugging routines for dumping file structures */
Expand Down
8 changes: 4 additions & 4 deletions src/H5ESint.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,10 +116,10 @@ bool H5_PKG_INIT_VAR = false;

/* Event Set ID class */
static const H5I_class_t H5I_EVENTSET_CLS[1] = {{
H5I_EVENTSET, /* ID class value */
0, /* Class flags */
0, /* # of reserved IDs for class */
(H5I_free_t)H5ES__close_cb /* Callback routine for closing objects of this class */
H5I_EVENTSET, /* ID class value */
0, /* Class flags */
0, /* # of reserved IDs for class */
H5ES__close_cb /* Callback routine for closing objects of this class */
}};

/* Declare a static free list to manage H5ES_t structs */
Expand Down
Loading
Loading