Bug 444278

Summary: SIGSEGV in musl freelocale
Product: [Developer tools] valgrind Reporter: shininggate83
Component: generalAssignee: Julian Seward <jseward>
Status: RESOLVED NOT A BUG    
Severity: normal CC: mark, pjfloyd, sam
Priority: NOR    
Version: unspecified   
Target Milestone: ---   
Platform: Other   
OS: Linux   
Latest Commit: Version Fixed In:
Sentry Crash Report:
Attachments: minimal test case
patch replacing __libc_free

Description shininggate83 2021-10-23 16:12:35 UTC
Created attachment 142798 [details]
minimal test case

SUMMARY
When running the program `min.c` under valgrind and musl libc, the process is terminated with SIGSEGV in `freelocale`.

STEPS TO REPRODUCE
1. Compile and run `min.c` with valgrind using musl libc.

OBSERVED RESULT
Program crashes with SIGSEGV.

EXPECTED RESULT
No crashes.

SOFTWARE/OS VERSIONS
Linux 5.10.75, Alpine Linux, musl 1.2.2

ADDITIONAL INFORMATION
The issue seems to be that musl calls `__libc_free` in freelocale, which valgrind does not replace, leading to musl trying to read metadata that is not present, instead getting garbage. In glibc, freelocale calls the normal `free` function, which is correctly replaced by valgrind.
Attached is a (semi-)tested patch which replaces `__libc_free`, making the test case and the original application run fine.
Comment 1 shininggate83 2021-10-23 16:13:14 UTC
Created attachment 142799 [details]
patch replacing __libc_free
Comment 2 Mark Wielaard 2021-10-24 11:47:27 UTC
Although it might be an idea to intercept __libc_free (but then also __libc_malloc) it seems you uncovered a bug in musl.

newlocale and freelocale do use __libc_malloc and __libc_free, but duplocale uses malloc. That means locales created by duplocale can use a different malloc allocator than the internal one. This bug would also show up if you used another LD_PRELOAD malloc ELF interposition tool.
Comment 3 Paul Floyd 2023-01-25 11:25:44 UTC
I agree with Mark. We shouldn't put bug workarounds in Valgrind unless really necessary.
Comment 4 Sam James 2023-01-25 21:12:36 UTC
I think this was fixed in musl by https://git.musl-libc.org/cgit/musl/commit/?id=6d8a515796270eb6cec8a278cb353a078a10f09a.
Comment 5 Paul Floyd 2023-01-26 06:39:35 UTC
Yes looks like it is fixed in musl.