Avoid memory corruption in arm context save/restore code#2276
Conversation
3efb186 to
613df32
Compare
|
|
||
| #undef ARM_ENV_RESTORE | ||
| // Overwrite uc to our uc | ||
| env->uc = uc; |
There was a problem hiding this comment.
Yes. We no longer clobber env->uc during restore, hence we do not need to fix env->uc to the correct uc after clobbering it with a potentially stale or mismatching pointer from the context.
|
LGTM except |
|
The reason is that AArch32 unicorn/qemu/target/arm/unicorn_arm.c Lines 440 to 443 in d4076e1
|
This points to a903fa1 Would you mind rebasing your branch to the latest dev so that we can have CI to run? |
2112f44 to
027015b
Compare
|
Rebased on latest |
|
mingw test failure is Github Action bug. |
I am not sure why thumb is a property of uc and not part of the CPU state, but thinking about it some more, the behaviour makes sense. I think this PR can go in as is (unless there are any concerns of course). |
Addresses multiple issues in the arm context save/restore code:
- When restoring from context, do not clobber pointers in CPUARMState
with data from the context buffer. This causes use-after-free bugs
usually manifesting as double-free crashes.
- When saving to context, do not copy pointers from CPUARMState to the
context buffer, to avoid leaking pointers to the context. Instead,
zero the respective area of the context buffer, to ensure that
reg_read and reg_write can access fields outside of the copied
range. Specifically, env->uc needs to be NULL when accessed for a
context.
- When restoring to a CPU with a different nr than the source CPU,
copy min(nr, ctx_nr) instead of nothing at all.
- Add regression tests checking all CPU models for arm and arm64 for
trivial double-free crashes.
Fixes unicorn-engine#2195
027015b to
cd473c7
Compare
|
Rebased on top of latest |
Addresses multiple issues in the arm context save/restore code:
Fixes #2195