[llvm-commits] [ASan] Replace CRT .dll malloc with our implementation at asan_init() time (issue 5708059)
Timur Iskhodzhanov
timurrrr at google.com
Wed Feb 29 05:52:47 PST 2012
Hi Aaron,
On Wed, Feb 29, 2012 at 5:45 PM, Aaron Ballman <aaron at aaronballman.com> wrote:
> On Wed, Feb 29, 2012 at 5:31 AM, <timurrrr at google.com> wrote:
>> Reviewers: glider,
>>
>> Message:
>> Hi Alexander,
>>
>> Can you please review this small CL?
>>
>> Thanks,
>> Timur
>>
>> Description:
>> Replace CRT .dll malloc with our implementation
>>
>> Please review this at http://codereview.appspot.com/5708059/
>>
>> Affected files:
>> M lib/asan/asan_malloc_win.cc
>>
>>
>> Index: lib/asan/asan_malloc_win.cc
>> diff --git a/lib/asan/asan_malloc_win.cc b/lib/asan/asan_malloc_win.cc
>> index
>> 40429f481e284221972754996284040f8ce9f7a3..42c54dc5d1818aeea6bddccf22df324a9a560a44
>> 100644
>> --- a/lib/asan/asan_malloc_win.cc
>> +++ b/lib/asan/asan_malloc_win.cc
>> @@ -18,12 +18,7 @@
>> #include "asan_internal.h"
>> #include "asan_stack.h"
>>
>> -namespace __asan {
>> -void ReplaceSystemMalloc() {
>> - // FIXME: investigate whether any action is needed.
>> - // Currenlty, we fail to intercept malloc() called from intercepted
>> _strdup().
>> -}
>> -} // namespace __asan
>> +#include "interception/interception.h"
>>
>> // ---------------------- Replacement functions ---------------- {{{1
>> using namespace __asan; // NOLINT
>> @@ -55,4 +50,37 @@ void *realloc(void *ptr, size_t size) {
>> }
>> } // extern "C"
>>
>> +using __interception::GetRealFunctionAddress;
>> +
>> +// We don't want to include "windows.h" in this file to avoid extra
>> attributes
>> +// set on malloc/free etc (e.g. dllimport), so declare a few things
>> manually:
>> +extern "C" int __stdcall VirtualProtect(void* addr, size_t size,
>> + DWORD prot, DWORD *old_prot);
>> +const int PAGE_EXECUTE_READWRITE = 0x40;
>> +
>> +namespace __asan {
>> +void ReplaceSystemMalloc() {
>> +#ifdef _WIN64
>> +# error ReplaceSystemMalloc was not tested on x64
>> +#endif
>> + char *crt_malloc;
>> + if (GetRealFunctionAddress("malloc", (void**)&crt_malloc)) {
>> + // Replace malloc in the CRT dll with a jump to our malloc.
>> + DWORD old_prot, unused;
>> + CHECK(VirtualProtect(crt_malloc, 16, PAGE_EXECUTE_READWRITE,
>> &old_prot));
>> + REAL(memset)(crt_malloc, 0xCC /* int 3 */, 16); // just in case.
>> +
>> + uintptr_t jmp_offset = (uintptr_t)malloc - (uintptr_t)crt_malloc - 5;
>> + crt_malloc[0] = 0xE9; // jmp, should be followed by an offset.
>> + REAL(memcpy)(crt_malloc + 1, &jmp_offset, sizeof(jmp_offset));
>
> Doing the memset to int 3s is not safe. You've not written your
> relative jump yet, so technically, something could be executing the
> code you're turning into int 3s.
You mean from some other thread?
I assume there's only one thread at the time this ReplaceSystemMalloc
is called [asan_init should be called before that moment].
Anyways, if something could execute "int 3s" while I'm writing jmp
then we have a dangerous data race and it's not safe to use not-atomic
memcpy either.
> Then you're always doing an absolute jump, destroying whatever was in
> the real malloc.
Yes, this was the intention.
> I believe malloc is hot-patchable already, so why not use the usual mechanisms?
I'm not sure I'm aware of hot-patching malloc.
Can you please elaborate?
> Is it acceptable to irretrievably destroy malloc for the session?
Ideally, we want "original" malloc/free to not be available at all
while running under ASan.
>> +
>> + CHECK(VirtualProtect(crt_malloc, 16, old_prot, &unused));
>> +
>> + // FYI: FlushInstructionCache is needed on Itanium etc but not on
>> x86/x64.
>> + }
>> +
>> + // FIXME: investigate whether anything else is needed.
>
> There's not, at least in terms of applying hot patches.
>
> ~Aaron
Thanks!
--
Timur
More information about the llvm-commits
mailing list