fix(macos): let a scan walk past a page whose pager declines to read - #88
Merged
Conversation
A scan of a live process aborted with MachReadError: mach_vm_read_overwrite failed: (os/kern) memory error (kr=10) part-way through, discarding the rest of the address space. kr=10 is KERN_MEMORY_ERROR, and mach/kern_return.h documents it as "During a page fault, the memory object indicated that the data could not be returned. This failure may be temporary; future attempts to access this same data may succeed" — in deliberate contrast with KERN_MEMORY_FAILURE (9) directly above it, whose comment ends "This failure is permanent." So by the kernel's own definition it belongs in _PAGE_GONE_KRS beside the three codes already there; it was simply missed, and wasn't even defined in types.py. The tolerance machinery was already in place and already wired into all three entry points — the code was just on the wrong side of the line _is_transient draws, so iter_search_results and iter_pattern_results re-raised it and killed the scan. It surfaces on file-backed read-only mappings — code segments, dylibs, the dyld shared cache — which is why a pattern scan hit it first: it deliberately ignores writeable_only (an AOB signature is normally in code), so it walks 2.3x the regions a default value scan does on a trivial process. Turning "writable regions only" off reproduces it with any value type. On this machine the whole memory/scan suite failed this way; it now passes, and those tests plant a value and assert they find it, so the scans are completing rather than quietly matching nothing.
…ther Self-review follow-ups, no behaviour change. The comment justifying the fix leaned on KERN_MEMORY_FAILURE (9) as the contrast case, but that constant wasn't in types.py — leaving a gap in the sequence (2, 4, 5, 8, 10) and a comment pointing at something the file didn't contain. The module's existing convention is to define the codes it talks about even when only prose refers to them (KERN_FAILURE is already there on those terms), so the two neighbours are now defined together with the header text that separates them. The test was declaring its own copy of 9, which is the kind of local constant that drifts from the library it describes. The "must still propagate" case tested KERN_SUCCESS, a state no MachReadError can carry — MachPartialReadError is raised on success but passes KERN_INVALID_ADDRESS. Swapped for KERN_FAILURE, which is what task_for_pid returns without the debugger entitlement and is a real error this must never swallow.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why is this PR necessary, what does it do?
A scan of a live process on macOS aborts part-way through with
throwing away the rest of the address space. The addresses found before the bad
page survive in the results table, so the app ends up in a confusing half-state:
a populated result list and an error dialog.
kr=10 is
KERN_MEMORY_ERROR, andmach/kern_return.hdocuments it as:— in deliberate contrast with
KERN_MEMORY_FAILURE(9) immediately above it inthat header, whose comment ends "This failure is permanent."
So by the kernel's own definition it belongs in
_PAGE_GONE_KRSalongside thethree codes already there. It was simply missed — it wasn't even defined in
macos/types.py.Nothing else needed changing. The tolerance machinery already exists and is
already wired into all three entry points (
search_addresses_by_value,search_addresses_by_pattern,search_values_by_addressesall passtransient_error_check=_is_transient). The code was just on the wrong side ofthe line
_is_transientdraws, soiter_search_resultsanditer_pattern_resultstook theirraisebranch and killed the scan.Why it shows up on pattern scans first
KERN_MEMORY_ERRORcomes from a pager declining to produce a page, so itturns up on file-backed read-only mappings — code segments, dylibs, the dyld
shared cache. A pattern scan deliberately ignores
writeable_only(an AOBsignature normally lives in code), so it walks those; a value scan with the
app's default "writable regions only" never goes near them. Measured on a
trivial Python process:
Turning "writable regions only" off reproduces it with any value type, which is
how this was confirmed rather than assumed.
Checklist (complete all items):
References:
No references to be shared.
Notes:
The new test pins both sides of the classification: the four codes a scan may
walk past, and the ones that must still reach the caller (a protection failure
or the permanent
KERN_MEMORY_FAILURE) so this doesn't drift into swallowingreal permission problems.
On the machine this was found on, the entire
tests/memoryandtests/scansuite failed this way — 15 tests, all with kr=10. They pass now. Worth noting
that they pass legitimately: those tests plant a value or a byte marker and
assert it is found (
assert address in hits), so the scans are completing andmatching, not quietly finding nothing.
Linux is unaffected — it has its own
_is_transientover a separate errno set.