[PR #430] [MERGED] fix(overlaybd): bound layer index ranges by file size #432

New issue

Closed

opened 2026-10-29 14:23:21 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-29 14:23:21 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/kvcache-ai/AgentENV/pull/430
Author: @yingdi-shan
Created: 10/23/2026
Status: ✅ Merged
Merged: 10/26/2026
Merged by: @yingdi-shan

Base: main ← Head: fix/overlaybd-index-bounds

📝 Commits (1)

d57b28b fix(overlaybd): bound layer index ranges by file size

📊 Changes

3 files changed (+128 additions, -6 deletions)

View changed files

📝 storage/overlaybd/src/compression/zfile.rs (+86 -5)
📝 storage/overlaybd/src/lsmt/file/helper.rs (+10 -1)
📝 storage/overlaybd/src/lsmt/file/tests.rs (+32 -0)

📄 Description

What

Harden index loading for sealed OverlayBD layers:

lsmt/file/helper.rs load_index: compute the index byte count with checked_mul and require index_offset + index_bytes <= file_size before allocating the index buffer. This covers every caller (LSMTReadOnlyFile::open, open_files_ro stacking, open_file_index, and the RW index reload).
compression/zfile.rs load_jump_table: apply the same index_offset / index_bytes bound in the overwrite-header branch that the trailer branch already applies.
compression/zfile.rs read_exact_chunks_at: use try_collect, so the parallel chunk read stops at the first failed chunk instead of driving every remaining chunk to completion.

Why

Both parsers trust the index size recorded in the layer's header or trailer and size allocations and reads from it, without checking it against the actual file length. A malformed layer should be rejected with an error at open time rather than consuming unbounded memory or CPU in the storage daemon.

N/A (reported privately)

Scope and non-goals

Only adds validation and fail-fast behavior on the read-only open paths. No format, layout, or write-path changes.
Does not change the zfile digest semantics (CALC_DIGEST remains optional, matching upstream OverlayBD).

Design and behavior changes

Malformed layers whose index range falls outside the file now fail to open with index range exceeds file size, index size overflow, or invalid index_offset beyond file end / invalid index bytes.
load_index makes one extra size() call per open. This is not on the write hot path; the existing size-call hot-path tests still pass.

Compatibility and operations

Public API or generated protocol: none.
Configuration or defaults: none.
Snapshot manifest, artifact layout, or storage format: none. Well-formed layers always satisfy the new bounds.
Upgrade and rollback: no migration needed.
Host requirements, permissions, ports, or dependencies: none.

Validation

make fmt
make clippy
make test-unit
Relevant Rust integration tests
make -C services test (required when services/ changes)
Generated clients/server regenerated with the documented make target
Documentation updated
Benchmarks or performance comparison completed

Commands and results:

make fmt                     # clean
make clippy                  # clean
cargo test -p overlaybd      # 405 passed, 0 failed (2 ignored), plus doc/bin targets ok

New tests:

lsmt::file::tests::test_readonly_open_rejects_index_outside_file
compression::zfile::tests::test_overwrite_header_rejects_index_outside_file (includes a positive control on an unmodified overwrite-header zfile)
compression::zfile::tests::test_read_exact_chunks_at_stops_after_first_error

The LSMT and chunk-reader tests were confirmed to fail with the source changes reverted.

Skipped checks and reasons:

make test-unit / make test-ublk: require privileged ublk/KVM setup. The change is confined to the overlaybd crate, whose full test suite was run.
services/, codegen, and docs: not touched.

Risks and reviewer notes

Low risk: the only behavior change is rejecting layers whose recorded index range does not fit inside the file.
Key files: storage/overlaybd/src/lsmt/file/helper.rs (load_index) and storage/overlaybd/src/compression/zfile.rs (load_jump_table, read_exact_chunks_at).

Checklist

The PR contains one coherent change and no unrelated formatting or refactoring.
New behavior is covered by tests, or I explained why testing is impractical.
Logs and examples contain no credentials, tokens, or private registry information.
I did not manually edit generated code without updating its source and regenerating it.

🤖 Generated with Claude Code

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/kvcache-ai/AgentENV/pull/430 **Author:** [@yingdi-shan](https://github.com/yingdi-shan) **Created:** 10/23/2026 **Status:** ✅ Merged **Merged:** 10/26/2026 **Merged by:** [@yingdi-shan](https://github.com/yingdi-shan) **Base:** `main` ← **Head:** `fix/overlaybd-index-bounds` --- ### 📝 Commits (1) - [`d57b28b`](https://github.com/kvcache-ai/AgentENV/commit/d57b28bcfc15c85f261f53db8f8a465e1146cf9e) fix(overlaybd): bound layer index ranges by file size ### 📊 Changes **3 files changed** (+128 additions, -6 deletions) <details> <summary>View changed files</summary> 📝 `storage/overlaybd/src/compression/zfile.rs` (+86 -5) 📝 `storage/overlaybd/src/lsmt/file/helper.rs` (+10 -1) 📝 `storage/overlaybd/src/lsmt/file/tests.rs` (+32 -0) </details> ### 📄 Description ## What Harden index loading for sealed OverlayBD layers: - `lsmt/file/helper.rs` `load_index`: compute the index byte count with `checked_mul` and require `index_offset + index_bytes <= file_size` before allocating the index buffer. This covers every caller (`LSMTReadOnlyFile::open`, `open_files_ro` stacking, `open_file_index`, and the RW index reload). - `compression/zfile.rs` `load_jump_table`: apply the same `index_offset` / `index_bytes` bound in the overwrite-header branch that the trailer branch already applies. - `compression/zfile.rs` `read_exact_chunks_at`: use `try_collect`, so the parallel chunk read stops at the first failed chunk instead of driving every remaining chunk to completion. ## Why Both parsers trust the index size recorded in the layer's header or trailer and size allocations and reads from it, without checking it against the actual file length. A malformed layer should be rejected with an error at open time rather than consuming unbounded memory or CPU in the storage daemon. ## Related issue N/A (reported privately) ## Scope and non-goals - Only adds validation and fail-fast behavior on the read-only open paths. No format, layout, or write-path changes. - Does not change the zfile digest semantics (`CALC_DIGEST` remains optional, matching upstream OverlayBD). ## Design and behavior changes - Malformed layers whose index range falls outside the file now fail to open with `index range exceeds file size`, `index size overflow`, or `invalid index_offset beyond file end` / `invalid index bytes`. - `load_index` makes one extra `size()` call per open. This is not on the write hot path; the existing size-call hot-path tests still pass. ## Compatibility and operations - Public API or generated protocol: none. - Configuration or defaults: none. - Snapshot manifest, artifact layout, or storage format: none. Well-formed layers always satisfy the new bounds. - Upgrade and rollback: no migration needed. - Host requirements, permissions, ports, or dependencies: none. ## Validation - [x] `make fmt` - [x] `make clippy` - [ ] `make test-unit` - [x] Relevant Rust integration tests - [ ] `make -C services test` (required when `services/` changes) - [ ] Generated clients/server regenerated with the documented `make` target - [ ] Documentation updated - [ ] Benchmarks or performance comparison completed Commands and results: ```text make fmt # clean make clippy # clean cargo test -p overlaybd # 405 passed, 0 failed (2 ignored), plus doc/bin targets ok ``` New tests: - `lsmt::file::tests::test_readonly_open_rejects_index_outside_file` - `compression::zfile::tests::test_overwrite_header_rejects_index_outside_file` (includes a positive control on an unmodified overwrite-header zfile) - `compression::zfile::tests::test_read_exact_chunks_at_stops_after_first_error` The LSMT and chunk-reader tests were confirmed to fail with the source changes reverted. Skipped checks and reasons: - `make test-unit` / `make test-ublk`: require privileged ublk/KVM setup. The change is confined to the `overlaybd` crate, whose full test suite was run. - `services/`, codegen, and docs: not touched. ## Risks and reviewer notes - Low risk: the only behavior change is rejecting layers whose recorded index range does not fit inside the file. - Key files: `storage/overlaybd/src/lsmt/file/helper.rs` (`load_index`) and `storage/overlaybd/src/compression/zfile.rs` (`load_jump_table`, `read_exact_chunks_at`). ## Checklist - [x] The PR contains one coherent change and no unrelated formatting or refactoring. - [x] New behavior is covered by tests, or I explained why testing is impractical. - [x] Logs and examples contain no credentials, tokens, or private registry information. - [x] I did not manually edit generated code without updating its source and regenerating it. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-29 14:23:21 +00:00