Skip to content

Handle short image reads#518

Merged
simsong merged 4 commits into
mainfrom
codex/issue-507-short-reads
Jul 20, 2026
Merged

Handle short image reads#518
simsong merged 4 commits into
mainfrom
codex/issue-507-short-reads

Conversation

@simsong-codex

Copy link
Copy Markdown
Collaborator

Codex-authored fix for #507.

Closes #507

What changed

  • Treat a positive raw-image short read as the actual buffer length rather than scanning the unread allocation tail.
  • Apply the same safe sbuf construction to EWF short reads and EOF.
  • Preserve the logical page size when a page plus margin is shortened.

Validation

  • make -s -j4 -C src check TESTS=test_be
  • The Linux test truncates an already-open raw image and verifies the reader exposes only the actual bytes. APFS retains the original view on an open descriptor, so that platform cannot reproduce this real short-read condition.

@codecov

codecov Bot commented Jul 20, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 97.01493% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 73.79%. Comparing base (5abbd86) to head (93fc1cc).
⚠️ Report is 5 commits behind head on main.

Files with missing lines Patch % Lines
src/image_process.cpp 91.67% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #518      +/-   ##
==========================================
+ Coverage   73.41%   73.79%   +0.38%     
==========================================
  Files         113      113              
  Lines       12586    12643      +57     
==========================================
+ Hits         9239     9329      +90     
+ Misses       3347     3314      -33     
Flag Coverage Δ
unittests 73.79% <97.01%> (+0.38%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@simsong-codex
simsong-codex force-pushed the codex/issue-507-short-reads branch from 2ec03ed to 466c3c1 Compare July 20, 2026 21:05
@simsong-codex
simsong-codex force-pushed the codex/issue-507-short-reads branch from 7e7cdbb to 93fc1cc Compare July 20, 2026 21:36
@simsong
simsong marked this pull request as ready for review July 20, 2026 21:52
Copilot AI review requested due to automatic review settings July 20, 2026 21:52
@simsong
simsong merged commit 71203c9 into main Jul 20, 2026
12 checks passed
@simsong
simsong deleted the codex/issue-507-short-reads branch July 20, 2026 21:52

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This pull request addresses issue #507 by making raw and EWF image readers handle positive short reads safely, ensuring scanners only see initialized bytes and that page-size semantics remain consistent when reads are truncated (e.g., due to EOF or live truncation).

Changes:

  • Introduces safe “shrink-to-actual-bytes-read” behavior for sbuf_t construction when a read returns fewer bytes than requested (raw + EWF).
  • Updates raw reader logic to use gcount() and treat positive short reads as valid buffers (and not scan unread allocation tail).
  • Adds regression tests for real-world raw truncation behavior on Linux and for EWF short-read/EOF behavior (when libewf is available).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
src/image_process.cpp Implements short-read-safe sbuf_t construction for EWF/raw readers and correct raw short-read handling via gcount().
src/test_be3.cpp Adds tests covering raw truncation short reads (Linux) and EWF short-read/EOF behavior (HAVE_LIBEWF).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/image_process.cpp
Comment on lines +49 to +55
static sbuf_t *shrink_sbuf(sbuf_t *sbuf, const pos0_t &pos0, size_t pagesize, size_t bytes_read)
{
auto *short_sbuf = sbuf_t::sbuf_malloc(pos0, bytes_read, std::min(pagesize, bytes_read));
memcpy(short_sbuf->malloc_buf(), sbuf->malloc_buf(), bytes_read);
delete sbuf;
return short_sbuf;
}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

P1: Handle short reads without scanning uninitialized bytes

3 participants