<div style="font-family: arial, helvetica, sans-serif; font-size: 10pt">Aha, cool solution! <div>Please make this a struct and copy-paste most of your message as a comment. </div><div><br></div><div>--kcc <br><br><div class="gmail_quote">
On Sat, Dec 1, 2012 at 9:29 PM, Alexander Potapenko <span dir="ltr"><<a href="mailto:glider@google.com" target="_blank">glider@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+<a href="mailto:jln@google.com">jln@google.com</a><br>
<br>
Thanks for your comments, I'll change those vars into a struct for sure.<br>
The intention behind this CL is to let ASan work with the setuid<br>
sandbox and the seccomp-bpf sandbox used in Chrome (in fact this<br>
particular changelist makes it happen).<br>
When a Chrome zygote is created, ASan runtime initializes itself<br>
(which includes reading /proc/self/maps), but then the sandboxes get<br>
enabled, which forbid further open() calls. That's why we're seeing<br>
the CHECK failures in the MemoryMappingLayout code soon after the<br>
program startup.<br>
Those sandboxes are designed in such a way (which is more or less<br>
natural to expect from any other sandbox filtering the file IO<br>
syscalls) that, once they are in play, the application is unable to<br>
load any new shared libraries. For this reason the Chrome zygote<br>
process loads all the libraries it's going to need before the<br>
sandboxing is turned on. Therefore it's safe to use the cached<br>
/proc/self/maps contents for now. We'll start experiencing problems<br>
with the cache again if we want ASan to parse the anonymous mappings<br>
in /p/s/m for some reason (I hope we won't ever need to, because this<br>
is not portable). Overall, a long-term solution we should consider is<br>
to add an ASan API that the client application will use to notify the<br>
tool about any sandbox to be used. It should be able to give ASan the<br>
pointers to proxied syscalls that ASan shall use in order to<br>
circumvent the sandboxing. This is something we shall discuss<br>
separately in detail, but which we fortunately don't need right now.<br>
<div class="HOEnZb"><div class="h5"><br>
On Sat, Dec 1, 2012 at 11:45 AM, Kostya Serebryany <<a href="mailto:kcc@google.com">kcc@google.com</a>> wrote:<br>
> Besides, I really don't understand what the intended caching behavior.<br>
> What of /proc/self/maps has changed and we are reading old data from cache?<br>
> And if we can request the contents of /proc/self/maps from the user, why do<br>
> we need cache?<br>
><br>
><br>
> On Sat, Dec 1, 2012 at 8:49 AM, Kostya Serebryany <<a href="mailto:kcc@google.com">kcc@google.com</a>> wrote:<br>
>><br>
>><br>
>><br>
>> On Sat, Dec 1, 2012 at 6:53 AM, Alexey Samsonov <<a href="mailto:samsonov@google.com">samsonov@google.com</a>><br>
>> wrote:<br>
>>><br>
>>><br>
>>><br>
>>> On Fri, Nov 30, 2012 at 6:39 PM, Alexander Potapenko <<a href="mailto:glider@google.com">glider@google.com</a>><br>
>>> wrote:<br>
>>>><br>
>>>> Author: glider<br>
>>>> Date: Fri Nov 30 20:39:45 2012<br>
>>>> New Revision: 169076<br>
>>>><br>
>>>> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=169076&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=169076&view=rev</a><br>
>>>> Log:<br>
>>>> Add caching to the MemoryMappingLayout class on Linux. This is necessary<br>
>>>> for the cases when a sandbox prevents ASan from reading the mappings<br>
>>>> from /proc/self/maps.<br>
>>>> The mappings are currently being cached on each access to<br>
>>>> /proc/self/maps. In the future we'll need to add an API that allows the<br>
>>>> client to notify ASan about the sandbox.<br>
>>>><br>
>>>><br>
>>>> Modified:<br>
>>>> compiler-rt/trunk/lib/sanitizer_common/sanitizer_linux.cc<br>
>>>> compiler-rt/trunk/lib/sanitizer_common/sanitizer_mac.cc<br>
>>>> compiler-rt/trunk/lib/sanitizer_common/sanitizer_procmaps.h<br>
>>>><br>
>>>> Modified: compiler-rt/trunk/lib/sanitizer_common/sanitizer_linux.cc<br>
>>>> URL:<br>
>>>> <a href="http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sanitizer_common/sanitizer_linux.cc?rev=169076&r1=169075&r2=169076&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sanitizer_common/sanitizer_linux.cc?rev=169076&r1=169075&r2=169076&view=diff</a><br>
>>>><br>
>>>> ==============================================================================<br>
>>>> --- compiler-rt/trunk/lib/sanitizer_common/sanitizer_linux.cc (original)<br>
>>>> +++ compiler-rt/trunk/lib/sanitizer_common/sanitizer_linux.cc Fri Nov 30<br>
>>>> 20:39:45 2012<br>
>>>> @@ -16,6 +16,7 @@<br>
>>>> #include "sanitizer_common.h"<br>
>>>> #include "sanitizer_internal_defs.h"<br>
>>>> #include "sanitizer_libc.h"<br>
>>>> +#include "sanitizer_mutex.h"<br>
>>>> #include "sanitizer_placement_new.h"<br>
>>>> #include "sanitizer_procmaps.h"<br>
>>>><br>
>>>> @@ -217,13 +218,23 @@<br>
>>>> }<br>
>>>><br>
>>>> // ----------------- sanitizer_procmaps.h<br>
>>>> +char *MemoryMappingLayout::cached_proc_self_maps_buff_ = NULL;<br>
>>>> +uptr MemoryMappingLayout::cached_proc_self_maps_buff_mmaped_size_ = 0;<br>
>>>> +uptr MemoryMappingLayout::cached_proc_self_maps_buff_len_ = 0;<br>
>>>> +StaticSpinMutex MemoryMappingLayout::cache_lock_;<br>
>>>> +<br>
>>>> MemoryMappingLayout::MemoryMappingLayout() {<br>
>>>> proc_self_maps_buff_len_ =<br>
>>>> ReadFileToBuffer("/proc/self/maps", &proc_self_maps_buff_,<br>
>>>> &proc_self_maps_buff_mmaped_size_, 1 << 26);<br>
>>>> - CHECK_GT(proc_self_maps_buff_len_, 0);<br>
>>>> + if (proc_self_maps_buff_mmaped_size_ == 0) {<br>
>>>> + LoadFromCache();<br>
>>>> + CHECK_GT(proc_self_maps_buff_len_, 0);<br>
>>>> + }<br>
>>>> // internal_write(2, proc_self_maps_buff_, proc_self_maps_buff_len_);<br>
>>>> Reset();<br>
>>>> + // FIXME: in the future we may want to cache the mappings on demand<br>
>>>> only.<br>
>>>> + CacheMemoryMappings();<br>
>>>> }<br>
>>>><br>
>>>> MemoryMappingLayout::~MemoryMappingLayout() {<br>
>>>> @@ -234,6 +245,39 @@<br>
>>>> current_ = proc_self_maps_buff_;<br>
>>>> }<br>
>>>><br>
>>>> +// static<br>
>>>> +void MemoryMappingLayout::CacheMemoryMappings() {<br>
>>>> + SpinMutexLock l(&cache_lock_);<br>
>>>> + // Don't invalidate the cache if the mappings are unavailable.<br>
>>>> + char *old_proc_self_maps_buff_ = cached_proc_self_maps_buff_;<br>
>>>> + uptr old_proc_self_maps_buff_mmaped_size_ =<br>
>>>> + cached_proc_self_maps_buff_mmaped_size_;<br>
>>>> + uptr old_proc_self_maps_buff_len_ = cached_proc_self_maps_buff_len_;<br>
>>>> + cached_proc_self_maps_buff_len_ =<br>
>>>> + ReadFileToBuffer("/proc/self/maps", &cached_proc_self_maps_buff_,<br>
>>>> + &cached_proc_self_maps_buff_mmaped_size_, 1 <<<br>
>>>> 26);<br>
>>>> + if (cached_proc_self_maps_buff_mmaped_size_ == 0) {<br>
>>>> + cached_proc_self_maps_buff_ = old_proc_self_maps_buff_;<br>
>>>> + cached_proc_self_maps_buff_mmaped_size_ =<br>
>>>> + old_proc_self_maps_buff_mmaped_size_;<br>
>>>> + cached_proc_self_maps_buff_len_ = old_proc_self_maps_buff_len_;<br>
>>>> + } else {<br>
>>>> + if (old_proc_self_maps_buff_mmaped_size_) {<br>
>>>> + UnmapOrDie(old_proc_self_maps_buff_,<br>
>>>> + old_proc_self_maps_buff_mmaped_size_);<br>
>>><br>
>>><br>
>>> OMG. I really think we need to wrap buff_, mmaped_size_ and buff_len_ in<br>
>>> struct.<br>
>><br>
>> +1<br>
>>><br>
>>><br>
>>>><br>
>>>> + }<br>
>>>> + }<br>
>>>> +}<br>
>>>> +<br>
>>>> +void MemoryMappingLayout::LoadFromCache() {<br>
>>>> + SpinMutexLock l(&cache_lock_);<br>
>>>> + if (cached_proc_self_maps_buff_) {<br>
>>>> + proc_self_maps_buff_ = cached_proc_self_maps_buff_;<br>
>>>> + proc_self_maps_buff_len_ = cached_proc_self_maps_buff_len_;<br>
>>>> + proc_self_maps_buff_mmaped_size_ =<br>
>>>> cached_proc_self_maps_buff_mmaped_size_;<br>
>>>> + }<br>
>>>> +}<br>
>>>> +<br>
>>>> // Parse a hex value in str and update str.<br>
>>>> static uptr ParseHex(char **str) {<br>
>>>> uptr x = 0;<br>
>>>><br>
>>>> Modified: compiler-rt/trunk/lib/sanitizer_common/sanitizer_mac.cc<br>
>>>> URL:<br>
>>>> <a href="http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sanitizer_common/sanitizer_mac.cc?rev=169076&r1=169075&r2=169076&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sanitizer_common/sanitizer_mac.cc?rev=169076&r1=169075&r2=169076&view=diff</a><br>
>>>><br>
>>>> ==============================================================================<br>
>>>> --- compiler-rt/trunk/lib/sanitizer_common/sanitizer_mac.cc (original)<br>
>>>> +++ compiler-rt/trunk/lib/sanitizer_common/sanitizer_mac.cc Fri Nov 30<br>
>>>> 20:39:45 2012<br>
>>>> @@ -162,6 +162,15 @@<br>
>>>> current_filetype_ = 0;<br>
>>>> }<br>
>>>><br>
>>>> +// static<br>
>>>> +void MemoryMappingLayout::CacheMemoryMappings() {<br>
>>>> + // No-op on Mac for now.<br>
>>>> +}<br>
>>>> +<br>
>>>> +void MemoryMappingLayout::LoadFromCache() {<br>
>>>> + // No-op on Mac for now.<br>
>>>> +}<br>
>>>> +<br>
>>>> // Next and NextSegmentLoad were inspired by base/sysinfo.cc in<br>
>>>> // Google Perftools, <a href="http://code.google.com/p/google-perftools" target="_blank">http://code.google.com/p/google-perftools</a>.<br>
>>>><br>
>>>><br>
>>>> Modified: compiler-rt/trunk/lib/sanitizer_common/sanitizer_procmaps.h<br>
>>>> URL:<br>
>>>> <a href="http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sanitizer_common/sanitizer_procmaps.h?rev=169076&r1=169075&r2=169076&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sanitizer_common/sanitizer_procmaps.h?rev=169076&r1=169075&r2=169076&view=diff</a><br>
>>>><br>
>>>> ==============================================================================<br>
>>>> --- compiler-rt/trunk/lib/sanitizer_common/sanitizer_procmaps.h<br>
>>>> (original)<br>
>>>> +++ compiler-rt/trunk/lib/sanitizer_common/sanitizer_procmaps.h Fri Nov<br>
>>>> 30 20:39:45 2012<br>
>>>> @@ -15,6 +15,7 @@<br>
>>>> #define SANITIZER_PROCMAPS_H<br>
>>>><br>
>>>> #include "sanitizer_internal_defs.h"<br>
>>>> +#include "sanitizer_mutex.h"<br>
>>>><br>
>>>> namespace __sanitizer {<br>
>>>><br>
>>>> @@ -39,9 +40,14 @@<br>
>>>> // address 'addr'. Returns true on success.<br>
>>>> bool GetObjectNameAndOffset(uptr addr, uptr *offset,<br>
>>>> char filename[], uptr filename_size);<br>
>>>> + // In some cases, e.g. when running under a sandbox on Linux, ASan is<br>
>>>> unable<br>
>>>> + // to obtain the memory mappings. It should fall back to pre-cached<br>
>>>> data<br>
>>>> + // instead of aborting.<br>
>>>> + static void CacheMemoryMappings();<br>
>>>> ~MemoryMappingLayout();<br>
>>>><br>
>>>> private:<br>
>>>> + void LoadFromCache();<br>
>>>> // Default implementation of GetObjectNameAndOffset.<br>
>>>> // Quite slow, because it iterates through the whole process map for<br>
>>>> each<br>
>>>> // lookup.<br>
>>>> @@ -77,6 +83,12 @@<br>
>>>> uptr proc_self_maps_buff_mmaped_size_;<br>
>>>> uptr proc_self_maps_buff_len_;<br>
>>>> char *current_;<br>
>>>> +<br>
>>>> + // Static mappings cache.<br>
>>>> + static char *cached_proc_self_maps_buff_;<br>
>>>> + static uptr cached_proc_self_maps_buff_mmaped_size_;<br>
>>>> + static uptr cached_proc_self_maps_buff_len_;<br>
>>>> + static StaticSpinMutex cache_lock_; // protects the cache contents.<br>
>>>> # elif defined __APPLE__<br>
>>>> template<u32 kLCSegment, typename SegmentCommand><br>
>>>> bool NextSegmentLoad(uptr *start, uptr *end, uptr *offset,<br>
>>>><br>
>>>><br>
>>>> _______________________________________________<br>
>>>> llvm-commits mailing list<br>
>>>> <a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
>>>> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
>>><br>
>>><br>
>>><br>
>>><br>
>>> --<br>
>>> Alexey Samsonov, MSK<br>
>>><br>
>>><br>
>>> _______________________________________________<br>
>>> llvm-commits mailing list<br>
>>> <a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
>>> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
>>><br>
>><br>
><br>
<br>
<br>
<br>
</div></div><span class="HOEnZb"><font color="#888888">--<br>
Alexander Potapenko<br>
Software Engineer<br>
Google Moscow<br>
</font></span></blockquote></div><br></div></div>