[llvm-commits] [ASan] Replace CRT .dll malloc with our implementation at asan_init() time (issue 5708059)

[Aaron Ballman]

On Wed, Feb 29, 2012 at 7:52 AM, Timur Iskhodzhanov <timurrrr at google.com> wrote:
> 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].

Correct

> 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.

Correct!  That's why hot patching was designed the way it was -- no data races.

>> 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?

Microsoft has hot patching functionality built in to a lot of their
APIs.  Basically, you'll see mov edi, edi as the first instruction in
a call, and the call will be preceded by 5 bytes of NOP instructions.
The basic idea is to write your absolute jump over the NOPs, then
write the single byte relative jmp over the mov edi, edi.  Et voila,
you've patched the function atomically.  And you can still call the
original by skipping over the relative jmp.

http://blogs.msdn.com/b/oldnewthing/archive/2011/09/21/10214405.aspx

I have code to do this, if interested/required.

~Aaron