Skip to content

Fix Clemory.split_backer destroying a nested clemory - #809

Open
zardus wants to merge 1 commit into
masterfrom
feature/clemory-split-nested
Open

Fix Clemory.split_backer destroying a nested clemory#809
zardus wants to merge 1 commit into
masterfrom
feature/clemory-split-nested

Conversation

@zardus

@zardus zardus commented Sep 3, 2026

Copy link
Copy Markdown
Member

THIS MESSAGE WAS GENERATED BY AN AUTOMATED PROCESS

Problem

Clemory.split_backer(addr) removes the wrong thing when addr falls inside a
nested clemory, and Loader.memory is exactly that shape -- Loader._map_object
adds a loaded object's memory to it as a nested clemory.

On master the damage is hidden behind #718 -- remove_backer never finds
its target, so the call raises ValueError: Can't find backer to remove and
nothing moves. With #718 applied it succeeds and silently drops memory. On
binaries/tests/x86_64/fauxware:

backers before: [('0x400000', 2676), ('0x600e28', 568), ('0x700000', 49)]
ld.memory.add_backer(ld.main_object.entry + 1, b"\x90" * 4, overwrite=True)
backers after : [('0x400000', 1409), ('0x400581', 4), ('0x400585', 1263), ('0x700000', 49)]

The 568-byte backer at 0x600e28 is gone -- 3293 backer bytes before, 2725
after -- and it is nowhere near the four bytes the caller asked to overwrite.

Root cause

split_backer finds its target with backers() and removes it with
remove_backer(), and those two do not look at the same collection. backers()
recurses into a nested clemory and yields the child's own bytearrays at absolute
addresses, while _backers, which remove_backer pops from, holds the child
clemory itself. So the split reads a start that names a backer inside the
child, removes the whole child, and puts back two slices of that one inner
backer.

The guard that was meant to cover this cannot fire:

if isinstance(backer, ClemoryBase):
    raise ValueError("Cannot split a backer which is itself a clemory")

backers() recurses past nested clemories rather than yielding them, so backer
is never a clemory.

Fix

Raise unless the backer backers() found is one of this clemory's own, which is
the precondition for remove_backer(start_addr) to remove the thing being split.
The guard stays where it is and the rest of the function is unchanged.

Raising rather than recursing into the child is deliberate. split_backer's only
caller in the tree is add_backer(overwrite=True), which afterwards calls
remove_backer on the outer clemory, and that can only remove from _backers. A
recursive split would therefore leave the outer holding both the child and the
new backer over the same addresses.

Testing

tests/test_clemory.py gains two tests, one on a nested clemory built in the
test and one on Loader.memory from binaries/tests/x86_64/fauxware. Both fail
on the merge base.

Nothing in cle, angr or angr-management reaches this on a load. Loading the 745
ELF files under binaries/tests calls split_backer 5,722 times and no call
gets as far as the removal, so the new guard never fires and no loaded object
changes. It is reachable through the public API, which is what the fauxware
example above uses.

Validation: #809 (comment)

session: sharpen

@zardus

zardus commented Sep 3, 2026

Copy link
Copy Markdown
Member Author

THIS MESSAGE WAS GENERATED BY AN AUTOMATED PROCESS

Validation record for head bf9d1c8add44930ad83e9ab683daef8ce8c7fd6c against baseline a96c36c18496d5c9177a995b9c7a5165fbc76217 (cle master), with fixtures from angr/binaries 45819e52da4b643383109eca2885a18c182486e2.

This head is 43e23fa2f0e73d70eff8e52964ec4d143cac5a5e with # type: ignore[arg-type] added to two lines of the new test, so ci / Typecheck stops counting them as new errors. The cle/ tree is byte-identical at both commits (05f22a2a63df7e4ef579385deedc640398e209b6), so the loader and reachability measurements below carry over unchanged; the suite and lint figures were re-measured at this head.

Measured configuration: a clone of cle at the revision named, Python 3.12.13, nice -n 19, no xdist, -p no:randomly. The baseline arm reverts only cle/memory.py to master and keeps the branch's tests, so the two arms differ by the production change alone. pylint is run with the CI configuration from angr/ci-settings, ci-image/conf/pylintrc, because cle declares no [tool.pylint] table.

  • Regression: python -m pytest -q -p no:randomly tests/test_clemory.py5 passed on this head, 2 failed, 3 passed on the baseline arm. The two failures are the tests this branch adds, test_split_backer_refuses_to_split_through_a_nested_clemory and test_split_backer_refuses_to_split_through_a_loaded_object; both get ValueError: Can't find backer to remove where the branch expects Cannot split a backer which is itself a clemory, because on the baseline the guard does not fire and remove_backer raises first
  • Full suite: python -m pytest -q -p no:randomly tests256 passed, 9 skipped on this head, 254 passed, 9 skipped on master
  • Loader non-regression: every tracked non-symlink file under binaries/tests whose first four bytes are \x7fELF745 objects — loaded with cle.Loader(path, auto_load_libs=False) on both arms, recording each loader's min_addr, max_addr, object count, the start and length of every backer from Loader.memory.backers(), and a SHA-256 over all backer bytes. 736 load on both arms and 9 are rejected on both, with the same three rejection classes (struct.error, AssertionError, ArchNotFound). Zero objects differ between the arms on any recorded field. binaries/tests holds 746 paths matching that test, but tests/armel/ld-linux-armhf.so.3 is a tracked symlink to tests/armhf/ld-linux-armhf.so.3, so 745 is the file count
  • Reachability, same 745 objects on the baseline arm: Clemory.split_backer was wrapped to classify each call before delegating. 5,722 calls, none of which reaches the removal — 5,708 return at StopIteration and 14 at addr <= start_addr. That is why the two arms are identical: the new guard cannot fire on any of these loads. All 736 loaders that load hold nested Clemory children in Loader.memory, 623 with two and 113 with one
  • Interaction with Fix Clemory backer removal and the BackedCGC backend #718: merging this head with #718 head b3e60a7d3c2b8f3681206ee8a84d34e4684ca3c8 is cleangit merge-tree --write-tree exits 0. The merged tree runs 261 passed, 9 skipped. On that merged tree the fauxware reproducer in the output comment raises and Loader.memory keeps all 3293 backer bytes; on #718 alone it returns and 2725 are left
  • Lint: ruff check and ruff format --check clean on both changed files. pylint per changed file, this head against master — cle/memory.py flat at 10.00, tests/test_clemory.py 5.12 -> 6.19. No file regresses
  • Typecheck, run as CI runs it (angr/ci-settings@master:ci-image/scripts/typecheck.py with pyright 1.1.411; it fails when any file changed in base...head has a higher whole-file pyright error count than at the base): cle master 3812052df2ad284cd16684fb7b7eb66e8d14dc6d against the merge of this head with it — cle/memory.py 0 -> 0, tests/test_clemory.py 9 -> 9, exit 0. The nine are pre-existing: four Clemory(None) calls, which the constructor annotates as archinfo.Arch, and five in the cffi test. The two Clemory(None) calls this branch adds carry # type: ignore[arg-type], which is what holds the count flat

Overlap and conflicts, measured against today's heads with git merge-tree --write-tree:

  • Make loader memory reads side-effect free #788 (d341e92824610e4400225ec697b97848c4fb64cb) rewrote split_backer to iterate self._backers, which made the same guard reachable, so its head fixed this defect as a side effect without mentioning it or testing it. It is now closed, so nothing else covers this defect and the conflict it had with this branch in cle/memory.py and tests/test_clemory.py no longer exists
  • Pack and unpack words struct cannot describe #721 (b5a0326b30f0c3fdfba508241ff24e372ee53f8a) conflicts in tests/test_clemory.py only; cle/memory.py merges clean and its split_backer is unchanged from master

Not run here: pre-commit run --all-files and the angr consumer suite. pre-commit.ci - pr and the hosted shards cover both at this head.

@zardus

zardus commented Sep 3, 2026

Copy link
Copy Markdown
Member Author

THIS MESSAGE WAS GENERATED BY AN AUTOMATED PROCESS

Full Loader.memory backer list for binaries/tests/x86_64/fauxware before and
after this change, either side of an overwriting add_backer one byte past the
entry point. Both sides are cle master merged with #718, because master
alone raises ValueError: Can't find backer to remove before it gets far enough
to do any damage.

import cle

ld = cle.Loader("binaries/tests/x86_64/fauxware", auto_load_libs=False)
print("nested children in Loader.memory:",
      sum(1 for _, b in ld.memory._backers if isinstance(b, cle.Clemory)))
print("backers before:", [(hex(s), len(b)) for s, b in ld.memory.backers()])
print("total backer bytes before:", sum(len(b) for _, b in ld.memory.backers()))
try:
    ld.memory.add_backer(ld.main_object.entry + 1, b"\x90" * 4, overwrite=True)
    print("add_backer(overwrite=True) returned")
except ValueError as e:
    print("add_backer(overwrite=True) raised ValueError:", e)
print("backers after :", [(hex(s), len(b)) for s, b in ld.memory.backers()])
print("total backer bytes after :", sum(len(b) for _, b in ld.memory.backers()))

Before — the overwrite drops the 568-byte backer at 0x600e28, which it was
never asked to touch:

#718 alone
nested children in Loader.memory: 2
backers before: [('0x400000', 2676), ('0x600e28', 568), ('0x700000', 49)]
total backer bytes before: 3293
add_backer(overwrite=True) returned
backers after : [('0x400000', 1409), ('0x400581', 4), ('0x400585', 1263), ('0x700000', 49)]
total backer bytes after : 2725

After — the overwrite is refused and the loader's memory is untouched:

with this change
nested children in Loader.memory: 2
backers before: [('0x400000', 2676), ('0x600e28', 568), ('0x700000', 49)]
total backer bytes before: 3293
add_backer(overwrite=True) raised ValueError: Cannot split a backer which is itself a clemory
backers after : [('0x400000', 2676), ('0x600e28', 568), ('0x700000', 49)]
total backer bytes after : 3293

@angr-bot

angr-bot commented Sep 3, 2026

Copy link
Copy Markdown
Member

Corpus decompilation diffs can be found at angr/dec-snapshots@master...angr/cle_809

split_backer finds its target with backers(), which recurses into nested
clemories and yields the child's own bytearrays, and then removes it with
remove_backer(), which only looks at self._backers, where the child itself
sits. Splitting on an address inside a child would remove the whole child
and put back two slices of one of the backers it held, dropping everything
else. On master remove_backer raises before that happens; with #718
applied it does not. Loader.memory is that shape, since a loaded object's
memory is a nested clemory inside it.

The guard meant to cover this tested the leaf backers() yielded, which is
never a clemory, so it could not fire. Raise unless the backer that was
found is one of this clemory's own, which is the precondition for
remove_backer to remove the thing being split.
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.

2 participants