Widen size checks instead of truncating to size_t - #372
Conversation
|
Thanks for these two nits! changed to We also have functions like |
0465b39 to
b190c5c
Compare
Check wire-format growth before allocation and propagate conversion failures through article readers and callers. Keep the historical tradspool article-size policy while storing representable lengths without narrowing.
Read large descriptors in representable chunks, preserve caller ownership and errno, and reject premature EOF. Keep active-file and buffer sizes tied to the stat used for their allocation.
Validate on-disk CAF bitmap, table, article, and recovery metadata before allocation or seeking. Preserve cleaner recovery for empty and repairable files while rejecting spans outside the backing file.
Route all failures after creating the PID-suffixed CAF path through one cleanup path. This closes the locked descriptor, unlinks the temporary file, and preserves the original errno for callers.
Retry interrupted and partial CAF metadata writes through the standard full-write helper instead of treating them as immediate I/O failures.
Make ovgroupunmap the sole owner of releasing per-group mappings. This removes duplicate munmap paths and lets partially constructed cache lists use the same cleanup path.
Reject unrepresentable or partial group.index files, preserve the current mapping until replacements succeed, propagate lookup failures, and support read-only remapping after concurrent growth.
A full freelist is represented by -1, so using the freelist head as the loop bound skipped hidden-group expiry once all mapped slots had been allocated. Refresh the mapping and scan every slot, relying on the zero count of unused entries to skip them.
Reject index and data sizes that cannot be represented by mapping and entry-count types. Preserve live mappings across remap and expansion failures, and audit the complete on-disk index after concurrent growth.
|
Good catch, that comparison was one instance of a broader problem. I audited the related In particular, the tradindexed paths now validate that file sizes are nonnegative and representable as I applied equivalent checks to buffindexed, including state-safe read-only remapping and deliberate rejection of partial trailing I force-pushed a cleaned-up nine-commit series so the review fixes are folded into their originating changes rather than left as follow-up commits. Most of these are low-value hardening changes that do not warrant backporting to stable; the possible exception is the buffindexed mapping-state work, which prevents silent |
|
Many thanks Kevin for this thorough review of |
|
Also, many thanks for the addition of new tests! It will help ensuring we do not have regressions between releases. |
Check wire-format growth before allocation and propagate conversion failures through article readers and callers. Keep the historical tradspool article-size policy while storing representable lengths without narrowing. see #372
Read large descriptors in representable chunks, preserve caller ownership and errno, and reject premature EOF. Keep active-file and buffer sizes tied to the stat used for their allocation. see #372
Validate on-disk CAF bitmap, table, article, and recovery metadata before allocation or seeking. Preserve cleaner recovery for empty and repairable files while rejecting spans outside the backing file. see #372
Route all failures after creating the PID-suffixed CAF path through one cleanup path. This closes the locked descriptor, unlinks the temporary file, and preserves the original errno for callers. see #372
Retry interrupted and partial CAF metadata writes through the standard full-write helper instead of treating them as immediate I/O failures. see #372
Make ovgroupunmap the sole owner of releasing per-group mappings. This removes duplicate munmap paths and lets partially constructed cache lists use the same cleanup path. see #372
Reject unrepresentable or partial group.index files, preserve the current mapping until replacements succeed, propagate lookup failures, and support read-only remapping after concurrent growth. see #372
A full freelist is represented by -1, so using the freelist head as the loop bound skipped hidden-group expiry once all mapped slots had been allocated. Refresh the mapping and scan every slot, relying on the zero count of unused entries to skip them. see #372
Julien-Elie
added
bug
2 participants
Casting off_t to unsigned int or size_t before comparing bounds drops high bits, so articles and CAF spans larger than the narrow type could pass validation. Compare via uintmax_t so the checks remain value-preserving.