Reject chunked datasets with mismatched chunk/dspace rank at open time#6508
Reject chunked datasets with mismatched chunk/dspace rank at open time#6508mattjala wants to merge 3 commits into
Conversation
H5D__chunk_construct() validates that the chunk layout dimensionality matches the dataspace rank, but that runs only at dataset creation time. When an existing dataset is opened, H5D__chunk_init() didn't repeat the check, so a file whose stored chunk rank disagreed with its dataspace rank was accepted. During chunk I/O the memory-selection rank (from the dataspace) and the file-selection rank (chunk ndims - 1) then differ, which produces a zero stride that causes a divide-by-zero in H5S__hyper_iter_get_seq_list(). H5D__chunk_init() now performs the same dimensionality check on open (the stored chunk rank includes the extra element-size dimension, so it must be exactly one greater than the dataspace rank) and rejects a mismatch with an error. Added test_chunk_dims_mismatch() as a regression test in test/dsets.c Fixes HDFGroup#6491
Review ChecklistThis PR touches the following areas. Each needs a sign-off
|
There was a problem hiding this comment.
Pull request overview
This pull request hardens chunked-dataset open-time validation to reject malformed files where the stored chunk layout dimensionality disagrees with the dataset dataspace rank, preventing a downstream divide-by-zero during chunk I/O selection iteration (Fixes #6491).
Changes:
- Add an open-time check in
H5D__chunk_init()to ensure stored chunk dimensionality matchesdataspace_rank + 1(datatype-size dimension). - Add a regression test (
test_chunk_dims_mismatch()) that verifies opening such a malformed chunked dataset fails cleanly. - Add a generator and CMake wiring for the malformed reference file, plus a changelog entry documenting the fix.
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
src/H5Dchunk.c |
Adds open-time validation to reject datasets with mismatched stored chunk ndims vs dataspace rank. |
test/dsets.c |
Adds a regression test that asserts malformed chunk-layout dimensionality is rejected on open. |
test/gen_bad_chunk.c |
Adds a generator program for producing a malformed chunked dataset file used by the regression test. |
test/CMakeTests.cmake |
Registers the new malformed reference file and generator executable in the CMake test configuration. |
release_docs/CHANGELOG.md |
Documents the crash fix and the new open-time validation behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
0ccb2cb to
daedfba
Compare
| * dimensionality is consistent with the dataspace rank. H5D__chunk_construct() | ||
| * performs this check at creation time, but it must also be performed | ||
| * here to catch malformed files. */ | ||
| if (open_op && dset->shared->layout.u.chunk.ndims != dset->shared->ndims + 1) |
There was a problem hiding this comment.
Is this check specific to this place in the dataset code? It seems like something that would be worth validating at the layout message decode time instead.
|
|
||
| ### Fixed a crash when reading a chunked dataset whose chunk rank mismatches the dataspace | ||
|
|
||
| The chunk layout's stored dimensionality was validated against the dataspace rank at creation time, but not at open time, so a file whose stored chunk rank disagrees with its dataspace rank would not be caught. The resulting inconsistent selection ranks during chunk I/O caused a divide-by-zero in the hyperslab iterator. The chunk dimensionality is now also validated on open, and such a dataset is rejected with an error instead of crashing. |
There was a problem hiding this comment.
disagrees -> disagreed
would not be caught -> was not caught
H5D__chunk_construct() only validates that the chunk layout dimensionality matches the dataspace rank at dataset creation time. When an existing dataset is opened, H5D__chunk_init() didn't repeat the check, so a file whose stored chunk rank disagreed with its dataspace rank was accepted.
During chunk I/O the memory-selection rank (from the dataspace) and the file-selection rank (chunk ndims - 1) then differ, which produces a zero stride that causes a divide-by-zero in H5S__hyper_iter_get_seq_list().
H5D__chunk_init() now performs the same dimensionality check on open (the stored chunk rank includes the extra element-size dimension, so it must be exactly one greater than the dataspace rank) and rejects a mismatch with an error.
Added test_chunk_dims_mismatch() as a regression test for this fix within test/dsets.c
Fixes #6491