This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

fortification and valgrind/memcheck (Was: [PATCH BZ#20422] Do not allow asan/msan/tsan and fortify@the same time)


On Tue, 2016-10-04 at 08:46 +0200, Jakub Jelinek wrote:
> On Mon, Oct 03, 2016 at 05:52:58PM -0700, Kostya Serebryany wrote:
> > >> Because you really don't know what kind of information will each tool want
> > >> to know, and that can significantly differ between valgrind, [amt]san etc.
> > >> In sanitizer_common, you can come up with some macros that will serve the
> > >> needs of all the tools, and have each tool use those macros, other than
> > >> that, it is a trivial 3 liner wrapper for each fortification function
> > 
> > Disagree. all the sanitizers will want to be directed to the original functions.
> > Valgrind most likely too.
> 
> At least in my understanding, valgrind doesn't want that, it wants to be
> able to nicely diagnose the error and for that it needs all the information
> about the reason.  It has the ability to override calls that don't go through
> PLT symbols already.

I haven't followed this thread but noticed it was mentioned in the bug
report "RFE: valgrind interoperation with fortification"
https://sourceware.org/bugzilla/show_bug.cgi?id=20644

I think Jakub is right, valgrind is fine with fortified code, it will
just be handled like any other code. The fortify checks might catch some
issues that valgrind/memcheck wouldn't because it has stricter bounds
(valgrind/memcheck only knows about memory blocks as a whole, so
wouldn't for example catch some buffer/stack overflows if it sees it as
one continuous block). On the other hand valgrind/memcheck will catch
other issues like using a recently freed block or dependencies not
initialized memory in the code.

So in general fortified code and valgrind memcheck compliment each
other. But valgrind could probably provide some extra information if it
understood why __chk_fail was called. Then it could intercept __chk_fail
and report what it knows about the address that was about to be accessed
(but that fortification prevented, so valgrind doesn't see it). For
example it could report where the block was created (with backtrace),
whether it was recently freed (with backtrace), how far beyond/inside a
block it was, etc.

My proposal was to extend __chk_fail (or introduce a new __chk_fail_addr
function) that provides the address that would have been accessed. Then
valgrind just intercepts __chk_fail and uses that address to provide
some additional information.

This might however be a slightly naive approach. It might create memory
pressure in the fast path because the address needs to be preserved
across the __chk_fail call (although the address was just checked, so is
probably easily available). It also might not provide enough information
(it doesn't say anything about the actual block/bounds that the user
intended to use, we just hope that the address is near enough for
valgrind to deduce the correct information).

Alternatively we might just add intercepts for all _chk functions and
write out the checks and report information ourselves. In theory this
would be easy, maybe a couple of days work since all those functions are
relatively trivial. Which would give much more accurate reports if a
fortification check triggers under valgrind. But there doesn't seem to
be an official list of _chk functions, it seems to involve just grepping
around in the glibc sources. And I am not sure how it would interact
with the gcc builtins
https://gcc.gnu.org/onlinedocs/gcc/Object-Size-Checking.html
We don't want to have to recompile a program to run under valgrind (the
code you run under valgrind should be what you would run in production
whenever possible). It seems we cannot intercept those builtin_chks and
have to rely on catching/intercepting the __chk_fail call they produce.
Which would call for adding the address/__chk_fail_addr again.

Cheers,

Mark


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]