mailing list of musl libc
 help / color / mirror / code / Atom feed
* Issue with musl and valgrind
@ 2020-01-06 11:01 d.dorau
  2020-01-06 13:56 ` Rich Felker
  0 siblings, 1 reply; 4+ messages in thread
From: d.dorau @ 2020-01-06 11:01 UTC (permalink / raw)
  To: musl

[-- Attachment #1: Type: text/plain, Size: 3003 bytes --]

Hello and a happy new year!

I'm looking for some help/advice regarding the use of valgrind with musl.
In my attempts to use valgrind with musl I ran into a reproducible crash. 
I found others
experienced this earlier as well, but with no apparent solution:

https://forum.archive.openwrt.org/viewtopic.php?id=70169

==3915== Invalid read of size 4
==3915==    at 0x48C3154: free (in 
/usr/lib/valgrind/vgpreload_memcheck-mips32-linux.so)
==3915==    by 0x4088C04: ??? (in /lib/libc.so)
==3915==  Address  is not stack'd, malloc'd or (recently) free'd

I did some debugging and came to a plausible reason, and with a quick
hack I could get it to run.

My findings are that when musl loads vgpreload_memcheck-mips32-linux.so,
malloc/free are replaced by the implementation provided from valgrind, as 
expected.

The free() call that leads to the crash originates from musl's 
dynlink.c:782 in
map_library().

The crash in valgrind's vgpreload_memcheck-mips32-linux.so is located in 
coregrind/m_replacemalloc/vg_replace_malloc.c:184 as part of the macro 
DO_INIT


static int init_done;
#define DO_INIT if (UNLIKELY(!init_done)) init()
[...]
#define FREE(soname, fnname, vg_replacement) \
   \
   void VG_REPLACE_FUNCTION_EZU(10050,soname,fnname) (void *p); \
   void VG_REPLACE_FUNCTION_EZU(10050,soname,fnname) (void *p)  \
   { \
      DO_INIT; \
      MALLOC_TRACE(#fnname "(%p)\n", p ); \
      if (p == NULL)  \
         return; \
      (void)VALGRIND_NON_SIMD_CALL1( info.tl_##vg_replacement, p ); \
   }

that is called in the free function provided by valgrind.

Calculating back the crash address 0x1e0d8 from the example above leads to 
the
address of the variable "init_done" of the valgrind code in the ELF file 
itself!

My interpretation is that although the relocation of the new free function 
has
apprently been done, the variables (and maybe other called functions) used 
*inside*
the valgrind library have not been relocated yet, so that their addresses 
still
point to the bare address in the ELF file. (I hope the explanation makes 
sense.)

I could avoid the crash by patching musl so that dynlink.c always calls 
musl's
own malloc/free function (by renaming and an additional wrapper for the 
exported
symbols).

Since I don't have in-depth knowledge about the inner workings of musl's 
shared
library loader, I'm now looking for advice on how to solve this issue.

At first glance the fact that functions inside the loaded library are 
called before
all relocations are done looks problematic to me. 
Assuming this would be solved, wouldn't it be desirable to ensure that 
malloc/free
calls inside musl won't be replaced by external libraries so that we don't 
get
stale allocations left from the old allocator?


Best regards,
Daniel





--
AVM Audiovisuelles Marketing und Computersysteme GmbH
Alt-Moabit 95, 10559 Berlin
HRB 23075 AG Charlottenburg
Geschäftsführer: Johannes Nill
 

[-- Attachment #2: Type: text/html, Size: 5564 bytes --]

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: Issue with musl and valgrind
  2020-01-06 11:01 Issue with musl and valgrind d.dorau
@ 2020-01-06 13:56 ` Rich Felker
  2020-01-06 16:24   ` Re[2]: " d.dorau
  0 siblings, 1 reply; 4+ messages in thread
From: Rich Felker @ 2020-01-06 13:56 UTC (permalink / raw)
  To: musl; +Cc: d.dorau

On Mon, Jan 06, 2020 at 12:01:34PM +0100, d.dorau@avm.de wrote:
> Hello and a happy new year!
> 
> I'm looking for some help/advice regarding the use of valgrind with musl.
> In my attempts to use valgrind with musl I ran into a reproducible crash. 
> I found others
> experienced this earlier as well, but with no apparent solution:
> 
> https://forum.archive.openwrt.org/viewtopic.php?id=70169
> 
> ==3915== Invalid read of size 4
> ==3915==    at 0x48C3154: free (in 
> /usr/lib/valgrind/vgpreload_memcheck-mips32-linux.so)
> ==3915==    by 0x4088C04: ??? (in /lib/libc.so)
> ==3915==  Address  is not stack'd, malloc'd or (recently) free'd
> 
> I did some debugging and came to a plausible reason, and with a quick
> hack I could get it to run.
> 
> My findings are that when musl loads vgpreload_memcheck-mips32-linux.so,
> malloc/free are replaced by the implementation provided from valgrind, as 
> expected.
> 
> The free() call that leads to the crash originates from musl's 
> dynlink.c:782 in
> map_library().
> 
> The crash in valgrind's vgpreload_memcheck-mips32-linux.so is located in 
> coregrind/m_replacemalloc/vg_replace_malloc.c:184 as part of the macro 
> DO_INIT
> 
> 
> static int init_done;
> #define DO_INIT if (UNLIKELY(!init_done)) init()
> [...]
> #define FREE(soname, fnname, vg_replacement) \
>    \
>    void VG_REPLACE_FUNCTION_EZU(10050,soname,fnname) (void *p); \
>    void VG_REPLACE_FUNCTION_EZU(10050,soname,fnname) (void *p)  \
>    { \
>       DO_INIT; \
>       MALLOC_TRACE(#fnname "(%p)\n", p ); \
>       if (p == NULL)  \
>          return; \
>       (void)VALGRIND_NON_SIMD_CALL1( info.tl_##vg_replacement, p ); \
>    }
> 
> that is called in the free function provided by valgrind.
> 
> Calculating back the crash address 0x1e0d8 from the example above leads to 
> the
> address of the variable "init_done" of the valgrind code in the ELF file 
> itself!
> 
> My interpretation is that although the relocation of the new free function 
> has
> apprently been done, the variables (and maybe other called functions) used 
> *inside*
> the valgrind library have not been relocated yet, so that their addresses 
> still
> point to the bare address in the ELF file. (I hope the explanation makes 
> sense.)
> 
> I could avoid the crash by patching musl so that dynlink.c always calls 
> musl's
> own malloc/free function (by renaming and an additional wrapper for the 
> exported
> symbols).
> 
> Since I don't have in-depth knowledge about the inner workings of musl's 
> shared
> library loader, I'm now looking for advice on how to solve this issue.
> 
> At first glance the fact that functions inside the loaded library are 
> called before
> all relocations are done looks problematic to me. 
> Assuming this would be solved, wouldn't it be desirable to ensure that 
> malloc/free
> calls inside musl won't be replaced by external libraries so that we don't 
> get
> stale allocations left from the old allocator?

This is really a bug in valgrind, that it's relying on a mix of the
dynamic linker and its own mechanism for interposing malloc. Either
the interposition should not take place until after line 1913 of
__dls3 in dynlink.c (this is what would happen if you did it via
LD_PRELOAD without valgrind), or valgrind should do its own loading
and relocation of vgpreload_memcheck-*.so independent of the tracee's
dynamic linker.

On most archs, the problem does not manifest as things are setup so
that it's safe to call free in an unrelocated context. However
realloc is gratuitously not safe and Adélie Linux has a patch for it
to do it like free:

https://code.foxkit.us/adelie/packages/blob/master/user/valgrind/realloc.patch

But on MIPS, where there are no PC-relative references and everything
goes through the GOT, this blows up.

I'm not sure how practical it is to get valgrind to fix this upstream.
In the either/or above, if the first course of action is taken, the
mechanism should probably be by inserting a breakpoint (or valgrind's
equivalent) at the main program's e_entry address and not doing the
replacement hack until then. Alternatively, it could just rely on
LD_PRELOAD working without trying to do additional tricks, but maybe
they have a good reason they're not doing that.

Rich


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re[2]: Issue with musl and valgrind
  2020-01-06 13:56 ` Rich Felker
@ 2020-01-06 16:24   ` d.dorau
  2020-01-06 16:32     ` Rich Felker
  0 siblings, 1 reply; 4+ messages in thread
From: d.dorau @ 2020-01-06 16:24 UTC (permalink / raw)
  To: Rich Felker; +Cc: musl

[-- Attachment #1: Type: text/plain, Size: 2550 bytes --]

"Rich Felker" <dalias@aerifal.cx> schrieb am 06.01.2020 14:56:23:
[...]
> 
> This is really a bug in valgrind, that it's relying on a mix of the
> dynamic linker and its own mechanism for interposing malloc. Either
> the interposition should not take place until after line 1913 of
> __dls3 in dynlink.c (this is what would happen if you did it via
> LD_PRELOAD without valgrind), or valgrind should do its own loading
> and relocation of vgpreload_memcheck-*.so independent of the tracee's
> dynamic linker.
> 
> On most archs, the problem does not manifest as things are setup so
> that it's safe to call free in an unrelocated context. However
> realloc is gratuitously not safe and Adélie Linux has a patch for it
> to do it like free:
> 
> 
https://code.foxkit.us/adelie/packages/blob/master/user/valgrind/realloc.patch

> 
> But on MIPS, where there are no PC-relative references and everything
> goes through the GOT, this blows up.
> 
> I'm not sure how practical it is to get valgrind to fix this upstream.
> In the either/or above, if the first course of action is taken, the
> mechanism should probably be by inserting a breakpoint (or valgrind's
> equivalent) at the main program's e_entry address and not doing the
> replacement hack until then. Alternatively, it could just rely on
> LD_PRELOAD working without trying to do additional tricks, but maybe
> they have a good reason they're not doing that.
> 
> Rich

I was not aware of this when debugging the crash, but after looking at 
additional
debug statements and valgrind code my current understanding is that 
valgrind
actually inserts

LD_PRELOAD="
/usr/lib/valgrind/vgpreload_core-mips32-linux.so:/usr/lib/valgrind/vgpreload_memcheck-mips32-linux.so"

into the environment before loading the executable to test.
(coregrind/m_initimg/initimg-linux.c:123)

425:11:32:696 --4757:1:    main Split up command line
425:11:32:696 --4757:1:    main (early_) Process Valgrind's command line 
options
425:11:32:696 --4757:1:    main Create initial image
425:11:32:712 --4757:1: initimg Loading client
425:11:32:712 --4757:1: initimg Setup client env
425:11:32:712 --4757:2: initimg   preload_string:
425:11:32:728 --4757:2: initimg 
"/usr/lib/valgrind/vgpreload_core-mips32-linux.so:/usr/lib/valgrind/vgpreload_memcheck-mips32-linux.so"
425:11:32:728 --4757:1: initimg Setup client stack: size will be 1048576


Is my understanding correct that you would expect it to not run into
the crash I described earlier then?


Daniel

[-- Attachment #2: Type: text/html, Size: 3606 bytes --]

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: Issue with musl and valgrind
  2020-01-06 16:24   ` Re[2]: " d.dorau
@ 2020-01-06 16:32     ` Rich Felker
  0 siblings, 0 replies; 4+ messages in thread
From: Rich Felker @ 2020-01-06 16:32 UTC (permalink / raw)
  To: d.dorau; +Cc: musl

On Mon, Jan 06, 2020 at 05:24:35PM +0100, d.dorau@avm.de wrote:
> "Rich Felker" <dalias@aerifal.cx> schrieb am 06.01.2020 14:56:23:
> [...]
> > 
> > This is really a bug in valgrind, that it's relying on a mix of the
> > dynamic linker and its own mechanism for interposing malloc. Either
> > the interposition should not take place until after line 1913 of
> > __dls3 in dynlink.c (this is what would happen if you did it via
> > LD_PRELOAD without valgrind), or valgrind should do its own loading
> > and relocation of vgpreload_memcheck-*.so independent of the tracee's
> > dynamic linker.
> > 
> > On most archs, the problem does not manifest as things are setup so
> > that it's safe to call free in an unrelocated context. However
> > realloc is gratuitously not safe and Adélie Linux has a patch for it
> > to do it like free:
> > 
> > 
> https://code.foxkit.us/adelie/packages/blob/master/user/valgrind/realloc.patch
> 
> > 
> > But on MIPS, where there are no PC-relative references and everything
> > goes through the GOT, this blows up.
> > 
> > I'm not sure how practical it is to get valgrind to fix this upstream.
> > In the either/or above, if the first course of action is taken, the
> > mechanism should probably be by inserting a breakpoint (or valgrind's
> > equivalent) at the main program's e_entry address and not doing the
> > replacement hack until then. Alternatively, it could just rely on
> > LD_PRELOAD working without trying to do additional tricks, but maybe
> > they have a good reason they're not doing that.
> > 
> > Rich
> 
> I was not aware of this when debugging the crash, but after looking at 
> additional
> debug statements and valgrind code my current understanding is that 
> valgrind
> actually inserts
> 
> LD_PRELOAD="
> /usr/lib/valgrind/vgpreload_core-mips32-linux.so:/usr/lib/valgrind/vgpreload_memcheck-mips32-linux.so"
> 
> into the environment before loading the executable to test.
> (coregrind/m_initimg/initimg-linux.c:123)
> 
> 425:11:32:696 --4757:1:    main Split up command line
> 425:11:32:696 --4757:1:    main (early_) Process Valgrind's command line 
> options
> 425:11:32:696 --4757:1:    main Create initial image
> 425:11:32:712 --4757:1: initimg Loading client
> 425:11:32:712 --4757:1: initimg Setup client env
> 425:11:32:712 --4757:2: initimg   preload_string:
> 425:11:32:728 --4757:2: initimg 
> "/usr/lib/valgrind/vgpreload_core-mips32-linux.so:/usr/lib/valgrind/vgpreload_memcheck-mips32-linux.so"
> 425:11:32:728 --4757:1: initimg Setup client stack: size will be 1048576
> 
> 
> Is my understanding correct that you would expect it to not run into
> the crash I described earlier then?

No, it does something explicitly wrong that causes the crash, and now
I'm starting to remember. What it does is hook the mmap of
vgpreload_memcheck-*.so and activate its redirections as soon as it's
mapped into memory. This activation should instead be moved to e_entry
point time.

BTW you accidentally un-CC'd the list; I bounced the email back on to
reply. Please keep replies on-list so others can follow.

Rich


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2020-01-06 16:32 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-06 11:01 Issue with musl and valgrind d.dorau
2020-01-06 13:56 ` Rich Felker
2020-01-06 16:24   ` Re[2]: " d.dorau
2020-01-06 16:32     ` Rich Felker

Code repositories for project(s) associated with this public inbox

	https://git.vuxu.org/mirror/musl/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).