[cfe-dev] annotating libc++ to catch buffer overflows in vector/string with asan

[Kostya Serebryany]

Here is another version; this time I've been able to build and run Chromium
with libc++ and these annotations.

--kcc


On Mon, Dec 9, 2013 at 6:58 PM, Kostya Serebryany <kcc at google.com> wrote:

> Marshall,
> Here is a more complete patch for asan vector annotations.
> It passes all tests in libcxx/test/containers/sequences/vector, but
> doesn't yet work on larger programs (I tried Chrome).
> Two questions:
>    1. Does it look like something potentially committable?
>    2. Would you suggest some bigger test suite specifically targeted to
> test std::vector?
>
> Thanks,
>
> --kcc
>
>
>
> On Wed, Nov 20, 2013 at 6:35 PM, Kostya Serebryany <kcc at google.com> wrote:
>
>> Attached is an intermediate patch that passes my tests (also attached).
>> Few questions:
>>
>> The patch gets a bit too verbose and it has a few repetitions, e.g. we
>> have this snippet repeated multiple times:
>> +            __annotate_contiguous_container(data(), data() + capacity(),
>> +                                            data() + size(), data() +
>> size() + 1);
>>
>> WDYT about adding a private method to vector like this (and a couple
>> more)?
>>
>> void __annotate_size_increase(size_t __n) {
>>     __annotate_contiguous_container(data(), data() + capacity(),
>>                                             data() + size(), data() +
>> size() + __n);
>> }
>>
>> In order to test the annotation properly I need to do something like this
>> as often as possible:
>> template <class T> void TestVector(std::vector<T> &v) {
>>
>>   if (v.capacity() == 0) return;
>>
>>   char *beg = (char*)&v.front();
>>
>>   char *end = beg + v.capacity() * sizeof(T);
>>
>>   char *mid = beg + v.size() * sizeof(T);
>>
>>   for (char *p = beg; p < mid; p++)
>>
>>     assert(!__asan_address_is_poisoned(p));
>>
>>   for (char *p = mid ; p < end; p++)
>>
>>     assert(__asan_address_is_poisoned(p));
>>
>> }
>>
>> Is it possible to add code like this as another private method of vector
>> (under some ifdef)?
>>
>> --kcc
>>
>>
>> On Tue, Nov 19, 2013 at 1:57 PM, Kostya Serebryany <kcc at google.com>wrote:
>>
>>>
>>>> > Do you like the idea?
>>>>
>>>> Yes - very much so.
>>>>
>>>> > Any comments about the prototype patch?
>>>>
>>>> Yes.
>>>>
>>>> 1) I would implement it with inline functions other than macros.
>>>>
>>>> Like this:
>>>>
>>>>
>>>> extern "C" void __sanitizer_annotate_contiguous_container(void *, void
>>>> *, void *, void *);
>>>> void inline __annotate_contiguous container
>>>>         ( const void *__beg, const void *__end, const void *__old_mid,
>>>> const void *new_mid )
>>>> #if __has_feature(address_sanitizer)
>>>>         {  __sanitizer_annotate_contiguous_container(beg, end, old_mid,
>>>> new_mid); }
>>>> #else
>>>>         {}
>>>> #endif
>>>>
>>>
>>> Makes sense, see below.
>>> We''l probably want to move __annotate_contiguous container into a
>>> separate file so that both string and vector use it.
>>> But let's do the whole thing with vector before moving to string.
>>>
>>>
>>>
>>>>
>>>> 2) I'd like more information about what
>>>> __sanitizer_annotate_contiguous_container does ;-)
>>>>
>>>
>>> In short, it [un]poisons the parts of shadow memory that correspond to
>>> the container's memory.
>>> More details in compiler-rt files:
>>>  include/sanitizer/common_interface_defs.h and lib/asan/asan_poisoning.cc
>>>
>>>
>>>
>>>>
>>>> > Any suggestion how to test these annotations in libc++ (we need some
>>>> kind of DEATH tests)?
>>>>
>>>> We already have failing tests in libc++.
>>>> they are named XXXX.fail.cpp.
>>>>
>>>
>>> These are tests that fail to compile.
>>> for asan annotations we need tests that build and run, and then either
>>> fail at run-time or check the state of shadow.
>>> Like these:
>>> compiler-rt/lib/asan/lit_tests/TestCases/contiguous_container.cc
>>> compiler-rt/lib/asan/lit_tests/TestCases/contiguous_container_crash.cc
>>>
>>> Suggestions?
>>>
>>> --kcc
>>>
>>> --- include/vector      (revision 195116)
>>> +++ include/vector      (working copy)
>>> @@ -288,6 +288,16 @@
>>>
>>>  _LIBCPP_BEGIN_NAMESPACE_STD
>>>
>>> +extern "C" void __sanitizer_annotate_contiguous_container(
>>> +  const void *, const void *, const void *, const void *);
>>> +void inline __annotate_contiguous_container
>>> +    (const void *__beg, const void *__end, const void *__old_mid, const
>>> void *__new_mid)
>>> +#if __has_feature(address_sanitizer)
>>> +    {  __sanitizer_annotate_contiguous_container(__beg, __end,
>>> __old_mid, __new_mid); }
>>> +#else
>>> +    {}
>>> +#endif
>>> +
>>>  template <bool>
>>>  class __vector_base_common
>>>  {
>>> @@ -1525,7 +1535,12 @@
>>>      // __v.push_back(_VSTD::forward<_Up>(__x));
>>>      __alloc_traits::construct(__a, _VSTD::__to_raw_pointer(__v.__end_),
>>> _VSTD::forward<_Up>(__x));
>>>      __v.__end_++;
>>> +    if (data())
>>> +      __annotate_contiguous_container(
>>>  +          data(), data() + capacity(), data() + size(), data() +
>>> capacity());
>>>      __swap_out_circular_buffer(__v);
>>> +    __annotate_contiguous_container(data(), data() + capacity(),
>>> +                                          data() + capacity(), data() +
>>> size());
>>>  }
>>>
>>>
>>>
>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20131210/681c8f08/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: vector_asan.patch
Type: text/x-patch
Size: 7635 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20131210/681c8f08/attachment.bin>