Validate metadata cache load sizes before allocation

[Alb3e3]

Describe your changes

This hardens malformed-file handling for metadata cache entry loads:

• Check non-speculative metadata cache entry image sizes against the file EOA before allocating the on-disk image buffer. Existing speculative load behavior is preserved: speculative reads can still be trimmed to EOA before retry.
• Validate decoded object header continuation chunk address/size pairs before queuing continuation chunks.
• Add an ohdr regression that creates an object header continuation chunk, corrupts the encoded continuation size on disk, and verifies the malformed object header is rejected before the cache allocates that size.

This prevents malformed metadata from driving very large allocations before the later file-read/EOA checks run.

AI assistance disclosure: this patch was prepared with assistance from OpenAI Codex. I reviewed the changes and ran the verification below locally.

Issue ticket number (GitHub or JIRA)

N/A

Checklist before requesting a review

• My code conforms to the guidelines in CONTRIBUTING.md
• I made an entry in release_docs/CHANGELOG.md (bug fixes, new features)
• I added a test (bug fixes, new features)

Local verification

• git diff --check origin/develop..HEAD
• cmake --build build-asan-o0 --target ohdr h5dump -j$(nproc)
• ASAN_OPTIONS=detect_leaks=0:allocator_may_return_null=0:abort_on_error=1 timeout 180s build-asan-o0/bin/ohdr
• 17 malformed h5dump crash-corpus files under ASan: no AddressSanitizer/UBSan/OOM output and no timeouts; all exit as ordinary open failures.
• Valid-file sanity check: build-asan-o0/bin/h5dump -H test/testfiles/tmtimeo.h5

[github-actions]

Review Checklist

This PR touches the following areas. Each needs a sign-off
from its listed owners before merging.

• src — @mattjala
• test — @mattjala
• release_docs — @hyoklee

[Alb3e3]

Pushed a follow-up for the CI failures. The first issue was formatting in test/ohdr.c. The Linux SWMR failures came from the new metadata load-size EOA guard rejecting reads against a SWMR reader's stale local EOA before the existing lower read path could handle writer-flushed metadata.

The update keeps the non-SWMR metadata size checks in place, removes the too-early continuation-message EOA check, and skips this cache-entry EOA validation only for H5F_ACC_SWMR_READ, matching the existing VFD/SWMR read behavior.

Local verification:

• git diff --check
• cmake --build /tmp/hdf5build-pr6506 --target flushrefresh swmr_check_compat_vfd ohdr use_disable_mdc_flushes twriteorder use_append_chunk use_append_mchunks -j2
• ctest -V -R '^H5SHELL-test_flush_refresh$'
• ctest -V -R '^H5SHELL-test_use_cases$'
• ./bin/ohdr

[hyoklee]

release_docs looks good.

[jhendersonHDF]

I'm somewhat suspicious about the need for this addition; does this become an issue only after the addition of the call to H5C__verify_len_eoa() in H5C__load_entry()? That sticks out to me as indicating that H5C__load_entry() may not be the place this issue needs to be solved at.

[Alb3e3]

Yes—the conflict was specific to applying the new non-speculative EOA check to SWMR readers. I kept the validation in H5C__load_entry() because this is the common point after the client computes len and immediately before allocating len + H5C_IMAGE_EXTRA_SPACE; validating later in a decoder would be after the allocation we need to bound. The SWMR exception is now scoped only to that new non-speculative call site, while the shared helper retains its pre-existing behavior for speculative clients. The focused cache and full ohdr binaries pass in a Debug build.

[fortnern]

Are there later checks that could be changed into asserts with this change? Or are there other paths to reach those later checks besides through H5C__load_entry()?

[Alb3e3]

Good questions — I looked at both.

Could later checks become asserts?

I'd recommend against it for the checks inside the client deserialize() callbacks, because this change doesn't make them unconditional:

• The SWMR carve-out means a non-speculative load under H5F_ACC_SWMR_READ still reaches H5MM_malloc() and deserialize() without an EOA check. Anything downgraded to an assert would become a release-build gap on exactly that path.
• For speculative loads the first pass still only gets the clamping form (actual = false); the strict form only runs afterwards if get_final_load_size() changes the length.

The only assert that would be sound is one in H5C__load_entry() itself right before the H5MM_malloc(), and that is tautological with the check I just added, so it buys nothing.

Are there other paths to those later checks?

Yes — H5C__deserialize_prefetched_entry() is a second type->deserialize() call site that never goes through H5C__load_entry(). It is the cache-image restore path, and it deliberately skips the load path's sizing logic; the comment above it says so:

Since the size of the on disk image is known exactly, there is no need for either a call to the get_initial_load_size() callback, or retries if the H5C__CLASS_SPECULATIVE_LOAD_FLAG flag is set. Similarly, there is no need to clamp possible reads beyond EOF.

There len = pf_entry_ptr->size, which is decoded from the cache image block — i.e. it comes from the file.

While checking that, I noticed the "known exactly" premise doesn't hold for a malformed file. In H5C__decode_cache_image_entry() on current develop:

H5F_addr_decode(f, &p, &pf_entry_ptr->addr);

/* Validate address range */
eoa = H5F_get_eoa(f, H5FD_MEM_DEFAULT);
if (!H5_addr_defined(pf_entry_ptr->addr) || H5_addr_overflow(pf_entry_ptr->addr, pf_entry_ptr->size) ||
    H5_addr_ge(pf_entry_ptr->addr + pf_entry_ptr->size, eoa))
    HGOTO_ERROR(H5E_CACHE, H5E_BADVALUE, NULL, "invalid entry address range");

/* Decode entry length */
H5F_DECODE_LENGTH(f, p, pf_entry_ptr->size);

The range check runs before the length is decoded. pf_entry_ptr comes from H5FL_CALLOC(), so ->size is still 0 there — the overflow and EOA terms are evaluated against zero and only the address is meaningfully validated. The real, file-supplied size is decoded on the next line and never re-checked against EOA.

That is not an out-of-bounds read in the decoder itself: the following H5MM_malloc() and image copy are bounded by H5_IS_BUFFER_OVERFLOW() against the cache image block. But it does mean a prefetched entry can carry a size that was never validated against EOA straight into type->deserialize() — which is the concrete reason I'd keep the client-side checks as real error checks rather than asserts.

Moving that validation to after the length decode looks like a small, self-contained fix. Happy to fold it into this PR or open a separate one — whichever you prefer.

[Alb3e3]

@mattjala Friendly ping — this PR has hyoklee's approval and the CI is green. Could you check off the src/test areas in the review checklist when you have a moment? Happy to address any remaining concerns.

[mattjala]

I think this can go in as it is now, though there's still a CHANGELOG conflict to resolve.