[tom-effernelli]

Fixes #2368.

The bug

When a UC_HOOK_MEM_WRITE_PROT hook makes the page writable with uc_mem_protect() and returns true, the write that triggered the hook is silently dropped; later writes land.

store_helper() reads the TLB entry into a local tlb_addr before running the hook. Toggling write access in uc_mem_protect() flushes the TLB (memory region commit → tcg_commit() → tlb_flush()), but the handled path only checks the stale local value, which still "hits", so the entry is never refilled and the stale TLB_DISCARD_WRITE drops the store. A uc_ctl_flush_tlb() inside the hook doesn't help, for the same reason.

load_helper() has the same pattern for UC_HOOK_MEM_READ_PROT: if the hook toggles write access (e.g. NONE → READ|WRITE), the load goes through the flushed entry's addend and can crash the host.

The fix

After a handled *_PROT hook, look the region up again (stop with UC_ERR_MAP if the hook unmapped it) and re-read the TLB entry, refilling it on a miss. This implements the contract documented in include/unicorn/unicorn.h: "returning true to continue execution will only work if the accessed memory is made accessible with the correct permissions during the hook."

Deliberately unchanged / out of scope:

• a hook that returns true without granting access still drops the write, as before;
• the *_UNMAPPED-handled branches, READ_PROT on the second page of split loads, and fetch (Fixing stale code tlb after fetch-prot hook #2365).

Tests

Three new tests in tests/unit/test_mem.c, which fail before the change (the first two lose the write, the third crashes with an access violation) and pass after:

• test_mem_protect_write_prot_hook_grant (the reproducer from writes to readonly mem get discarded when MEM_WRITE_PROT hook returns true #2368)
• test_mem_protect_write_prot_hook_grant_cross_page (a store split across a writable and a read-only page)
• test_mem_protect_read_prot_hook_grant

Tested locally on Windows with MinGW (MSYS2 gcc), x86 only: test_mem, test_x86 and test_ctl pass (except test_mem_read_and_write_large_memory_block, which needs ARM64, not built). I'm relying on CI for the other targets.

Note on #2415

Draft PR #2415 by @kibertoad addresses this more broadly, and also changes the hook semantics (returning true performs the access even without granting it). This PR is a minimal, behaviour-preserving fix offered as something small to review and land in the meantime; I'm happy to close it if you prefer #2415.

This change was prepared with the help of Claude Code.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Sdo1fYpWGRmybV5et7sDxu