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

Fix a memory corruption issue in H5S__point_project_simple #2626

Merged
merged 1 commit into from
Mar 24, 2023
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
14 changes: 14 additions & 0 deletions release_docs/RELEASE.txt
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,20 @@ Bug Fixes since HDF5-1.13.3 release
===================================
Library
-------
- Fixed a memory corruption issue that can occur when reading
from a dataset using a hyperslab selection in the file
dataspace and a point selection in the memory dataspace

When reading from a dataset using a hyperslab selection in
the dataset's file dataspace and a point selection in the
dataset's memory dataspace where the file dataspace's "rank"
is greater than the memory dataspace's "rank", memory corruption
could occur due to an incorrect number of selection points
being copied when projecting the point selection onto the
hyperslab selection's dataspace.

(JTH - 2023/03/23)

- Fixed issues in the Subfiling VFD when using the SELECT_IOC_EVERY_NTH_RANK
or SELECT_IOC_TOTAL I/O concentrator selection strategies

Expand Down
2 changes: 1 addition & 1 deletion src/H5Spoint.c
Original file line number Diff line number Diff line change
Expand Up @@ -2319,7 +2319,7 @@ H5S__point_project_simple(const H5S_t *base_space, H5S_t *new_space, hsize_t *of
/* Copy over the point's coordinates */
HDmemset(new_node->pnt, 0, sizeof(hsize_t) * rank_diff);
H5MM_memcpy(&new_node->pnt[rank_diff], base_node->pnt,
(new_space->extent.rank * sizeof(hsize_t)));
(base_space->extent.rank * sizeof(hsize_t)));
Copy link
Collaborator Author

@jhendersonHDF jhendersonHDF Mar 23, 2023

Choose a reason for hiding this comment

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

If you expand the code above, you can see that this block assumes new_space->extent.rank > base_space->extent.rank. This copying code allocates a new_space->extent.rank-sized array of hsize_t for the point coordinates, then offsets into the pnt array by rank_diff which is calculated by new_space->extent.rank - base_space->extent.rank. However, it previously tried copying a new_space->extent.rank amount of points into the array AFTER offsetting into the array, whereas there would only be room left for a base_space->extent.rank amount of points. This issue cropped up with a Developer build that disables free lists, so this issue has likely been around for a long time but went unnoticed because the free lists were allocating enough room that we could happily write over.

Copy link
Contributor

Choose a reason for hiding this comment

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

Nice catch!


/* Keep the order the same when copying */
if (NULL == prev_node)
Expand Down