Re: [PATCH BZ#20422] Do not allow asan/msan/tsan and fortify at the same time.

[Andrew Pinski <pinskia at gmail dot com>]

On Thu, Sep 29, 2016 at 2:23 PM, Kostya Serebryany <kcc@google.com> wrote:
> 80 interceptors to support *san and fortification is 80 too many, IMHO.
> The fact that other pre-compiled libraries use fortify by default is very sad.
> I think this is a clear case of misuse of fortify because now users of
> the library can't opt out.

Why do you think this is very sad?  This is not a misuse of fortify
but rather santizers not adapting to the changing environments and
handling how people are treating security issues now.  Remember 10
years ago there was no such thing as fortification and security was
not treated as a first class citizen.  Now Security is treated as a
first class and people are requiring security before anything else.

Thanks,
Andrew Pinski


>
> If someone is willing to provide a patch to sanitizers that is
>   * in sanitizer_common,
>   * uses a separate file for all of these 80 functions,
>   * does not touch any other file (in a significant way, at least)
>   * has tests
> I'll most likely accept it.
>
> (Yuri, thanks for all this work!  )
>
> --kcc
>
>
> On Thu, Sep 29, 2016 at 3:52 AM, Andrew Pinski <pinskia@gmail.com> wrote:
>> On Thu, Sep 29, 2016 at 6:44 PM, Jakub Jelinek <jakub@redhat.com> wrote:
>>> On Thu, Sep 29, 2016 at 11:32:19AM +0100, Yuri Gribov wrote:
>>>> > Ugh, so you want to slow down the performance critical common path of some
>>>> > of the most often used functions just to avoid adding small code to the
>>>> > sanitizers and valgrind?
>>>>
>>>> Well, I think it's not just ASan and valgrind but rather all (or most
>>>> of) dynamic instrumentation tools (TSan, MSan, dmalloc, etc.). We
>>>> suggest to handle forwarding in one place rather than ask all tools
>>>> developers to re-implement it.
>>>
>>> ASan/TSan/MSan can all share the same implementation, like they share
>>> various other wrappers.  And the needs of the different tools are so
>>> different that adding a preload DSO that just ignores it will not help at
>>> all, usually you really don't want to ignore it, but complain using your
>>> tools diagnostic framework, for that you need to know what argument is
>>> objsz, against what it is compared (some other argument, strlen of some
>>> argument, something else) etc.
>>
>> Plus the other argument doing it not in glibc is that some distros are
>> not going to support this until much later so it will be still broken
>> on those.  Plus what about FreeBSD, etc.
>>
>>>
>>>> > Why?  There are not many of the __*_chk entrypoints, and it is fairly easy
>>>> > to handle them.
>>>>
>>>> I can only access 2.12 now (yeah...) but it has 90+ *_chk APIs. Surely
>>>> not all are necessary but that's still a lot of new interceptors.
>>>
>>> So what?  In most cases with sufficient macros you'll need just one or three
>>> lines for each.  Basically return type, function name, arguments and their
>>> types, how they map to the underlying non-fortified fn, what is the objsz
>>> argument and against what it is compared.
>>> BTW, I only count 80 on my box with glibc 2.21:
>>> nm -D /lib64/libc.so.6 | awk '/_chk/{print $3}' | sort -u | wc -l
>>
>> And for 2.23 on ARM64 I get only 81 (well 80 if you don't count
>> __chk_fail which the above command still counts).
>> In one year none was added.
>>
>>
>> Thanks,
>> Andrew Pinski
>>
>>>
>>>> > E.g. asan has hundreds to thousands of wrappers, and most
>>>> > of the *_chk entrypoints could be easily macrofied and the tool (asan,
>>>> > valgrind) can then decide what it prefers to do with them (ignore the __bos
>>>> > supplied argument, or add another diagnostics for the UB, etc.).
>>>>
>>>> It's not just about implementing them but also synchronizing when
>>>> Glibc adds new fortified implementations or changes their interfaces.
>>>
>>> It adds them very rarely, and never changes, it has to maintain the ABI.
>>>
>>>         Jakub