[PATCH] D137292: [lsan] Fix stack buffer overwrite in SuspendedThreadsListMac::GetRegistersAndSP

[Kuba (Brecka) Mracek via Phabricator via llvm-commits]

kubamracek added inline comments.


================
Comment at: compiler-rt/lib/sanitizer_common/sanitizer_stoptheworld_mac.cpp:152
   int err;
-  mach_msg_type_number_t reg_count = MACHINE_THREAD_STATE_COUNT;
-  err = thread_get_state(thread, MACHINE_THREAD_STATE, (thread_state_t)&regs,
+  mach_msg_type_number_t reg_count = sizeof(regs) / sizeof(natural_t);
+  err = thread_get_state(thread, regs_flavor, (thread_state_t)&regs,
----------------
usama54321 wrote:
> I will just suggest using the corresponding macros for reg count, e.g. ARM_THREAD_STATE64_COUNT etc. from the headers rather the inlining the calculation. Apart from that looks good to me.
I think I'm slightly against that (I considered it initially) because this is exactly what caused the bug in the first place (!) and triggered a stack smasher (!!!) with very non-obvious consequences. The reg_count variable is telling thread_get_state how large the buffer is that it can write to... and I'd say that the most correct and most obvious way of getting that right is by actually using sizeof on the actual stack variable that holds the buffer.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137292/new/

https://reviews.llvm.org/D137292