Matthew McGovern via llvm-dev
2019-May-23 18:52 UTC
[llvm-dev] [sanitizers][windows] adding RtlAllocateHeap interception
Windows sanitizer community, I have a patch to add interception for the Rtl*Heap family of allocation functions. The change requires some additions to the existing regular Heap* interceptors since we are now also dealing with allocations that were created very early on during initialization. The current source code mentions why this isn't ideal: // We don't currently intercept all calls to HeapAlloc. If we did, we would have to check on // HeapFree whether the pointer came from ASan or from the system. SO... We implemented it and it does get sort of gross, especially in the ReAllocate case. There's also the issue of RtlSizeHeap and RtlReAllocateHeap being undocumented (for now). Some Rtl*Heap functions are documented in the Windows Driver development kit and it's possible that we can have ReAlloc and Size added. Because of all the previously mentioned issues I would like to add this interception code behind a preprocessor macro. I'm guessing everyone will not neccesarily need it and the code will introduce a performance penalty when enabled. Does the community have any input before I push this patch out for review? Do you think there is a better approach for hiding this code, do you think it should be a runtime option instead? Is there anyone opposed to having this in the compiler-rt windows source for some reason I haven't considered? Thank you all for your time and input, Matthew G McGovern | Security Software Engineer Microsoft COSINE | Platform Security and Vulnerabilty Research One Microsoft Way | Redmond, WA 98052 -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20190523/2f217bbc/attachment.html>
Reid Kleckner via llvm-dev
2019-Jun-24 20:52 UTC
[llvm-dev] [sanitizers][windows] adding RtlAllocateHeap interception
(Super delayed reply from after you sent the patches) I think in general runtime flags are preferred to build configurations and macros. At this point, you've probably spent more time than most people measuring Windows ASan performance, and any data you have is certainly the freshest. If you think the extra branches and heap ownership checks are acceptable and it doesn't affect the main `free` codepath on Linux where performance is more closely monitored, then it should be acceptable. On Thu, May 23, 2019 at 11:52 AM Matthew McGovern < Matthew.Mcgovern at microsoft.com> wrote:> Windows sanitizer community, > > > > I have a patch to add interception for the Rtl*Heap family of allocation > functions. The change requires some additions to the existing regular > Heap* interceptors since we are now also dealing with allocations that were > created very early on during initialization. > > > > The current source code mentions why this isn’t ideal: > > > > *// We don't currently intercept all calls to HeapAlloc. If we did, we > would have to check on* > > *// HeapFree whether the pointer came from ASan or from the system.* > > > > SO… We implemented it and it does get sort of gross, especially in the > ReAllocate case. There’s also the issue of RtlSizeHeap and > RtlReAllocateHeap being undocumented (for now). Some Rtl*Heap functions > are documented in the Windows Driver development kit and it’s possible that > we can have ReAlloc and Size added. > > > > Because of all the previously mentioned issues I would like to add this > interception code behind a preprocessor macro. I’m guessing everyone will > not neccesarily need it and the code will introduce a performance penalty > when enabled. > > > > Does the community have any input before I push this patch out for review? > Do you think there is a better approach for hiding this code, do you think > it should be a runtime option instead? Is there anyone opposed to having > this in the compiler-rt windows source for some reason I haven’t considered? > > > > Thank you all for your time and input, > > > > Matthew G McGovern | Security Software Engineer > > Microsoft COSINE | Platform Security and Vulnerabilty Research > > One Microsoft Way | Redmond, WA 98052 > > >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20190624/3c38edfb/attachment.html>