| Message ID | 20210702113845.3367306-1-siddhesh@sourceware.org |
|---|---|
| Headers | show |
| Series | Remove malloc hooks | expand |
On 7/2/21 7:38 AM, Siddhesh Poyarekar wrote: > This patchset removes the malloc hooks __malloc_hook, __free_hook, > __realloc_hook and __memalign_hook from the API and leaves compatibility > symbols so that existing applications can continue to link to them. The > reading and execution of the hooks has been moved to a DSO > libmalloc_compathooks.so, which can be preloaded for applications that > need it. By default these hooks no longer have any effect in the > library. OK. Good. > Further, the __morecore, __morecore_after_hook and __default_morecore > hooks have also been moved to compat symbols and removed from the API. > Existing applications will continue to link to them but they won't have > any effect on malloc behaviour. OK. Good. > To enable this, the MALLOC_CHECK_, mcheck() and mtrace() hooks have been > weaned away from hooks. In the process, some mcheck() failures have > been fixed and the overall behaviour is now simpler to follow. Finally, > tr_break and mallwatch symbols have been deprecated and are not used > anywhere. Users are advised to use gdb watchpoints and conditional > breakpoints to debug malloc internals since they ought to provide > equivalent functionality. OK. Thanks for working through this. This is looking better but we need to discuss the mcheck status and dropping some of those interfaces, perhaps just deprecating libmcheck.a and moving the functionality into libc_malloc_debug.so. See further review. I think we're almost done with a v4 review if we agree on direction. > Changes from v3: > - Remove source file dependencies > - Commit mcheck tests > > Changes from v2: > - Move hooks dependencies to malloc.o{,sS} > > Changes from v1: > > - Added makefile dependencies for the new hooks files > - Fixed memset call in calloc debugging hooks > - Added the tr_break deprecation patch and mcheck test patch to this > series > > Siddhesh Poyarekar (10): > Drop source dependencies on hooks.c and arena.c > mtrace: Deprecate mallwatch and tr_break > Move glibc.malloc.check implementation into its own file > malloc: Move malloc hook references to hooks.c > glibc.malloc.check: Wean away from malloc hooks > mcheck: Wean away from malloc hooks > mtrace: Wean away from malloc hooks > Remove malloc hooks > Remove __after_morecore_hook > Remove __morecore and __default_morecore > > Makeconfig | 2 +- > NEWS | 18 + > include/malloc.h | 11 +- > include/stdlib.h | 3 - > malloc/Makefile | 23 +- > malloc/Versions | 3 + > malloc/arena.c | 26 +- > malloc/hooks.c | 545 +++++++----------- > malloc/malloc-check.c | 376 ++++++++++++ > malloc/malloc-compathooks.c | 166 ++++++ > malloc/malloc-internal.h | 6 + > malloc/malloc.c | 235 ++++---- > malloc/malloc.h | 27 - > malloc/mcheck-hooks.c | 411 +++++++++++++ > malloc/mcheck-init.c | 14 +- > malloc/mcheck.c | 369 +----------- > malloc/morecore.c | 15 +- > malloc/mtrace-hooks.c | 137 +++++ > malloc/mtrace.c | 276 +-------- > manual/memory.texi | 191 +----- > sysdeps/mach/hurd/i386/libc.abilist | 1 + > sysdeps/unix/sysv/linux/aarch64/libc.abilist | 1 + > sysdeps/unix/sysv/linux/alpha/libc.abilist | 1 + > sysdeps/unix/sysv/linux/arc/libc.abilist | 1 + > sysdeps/unix/sysv/linux/arm/be/libc.abilist | 1 + > sysdeps/unix/sysv/linux/arm/le/libc.abilist | 1 + > sysdeps/unix/sysv/linux/csky/libc.abilist | 1 + > sysdeps/unix/sysv/linux/hppa/libc.abilist | 1 + > sysdeps/unix/sysv/linux/i386/libc.abilist | 1 + > sysdeps/unix/sysv/linux/ia64/libc.abilist | 1 + > .../sysv/linux/m68k/coldfire/libc.abilist | 1 + > .../unix/sysv/linux/m68k/m680x0/libc.abilist | 1 + > .../sysv/linux/microblaze/be/libc.abilist | 1 + > .../sysv/linux/microblaze/le/libc.abilist | 1 + > .../sysv/linux/mips/mips32/fpu/libc.abilist | 1 + > .../sysv/linux/mips/mips32/nofpu/libc.abilist | 1 + > .../sysv/linux/mips/mips64/n32/libc.abilist | 1 + > .../sysv/linux/mips/mips64/n64/libc.abilist | 1 + > sysdeps/unix/sysv/linux/nios2/libc.abilist | 1 + > .../linux/powerpc/powerpc32/fpu/libc.abilist | 1 + > .../powerpc/powerpc32/nofpu/libc.abilist | 1 + > .../linux/powerpc/powerpc64/be/libc.abilist | 1 + > .../linux/powerpc/powerpc64/le/libc.abilist | 1 + > .../unix/sysv/linux/riscv/rv32/libc.abilist | 1 + > .../unix/sysv/linux/riscv/rv64/libc.abilist | 1 + > .../unix/sysv/linux/s390/s390-32/libc.abilist | 1 + > .../unix/sysv/linux/s390/s390-64/libc.abilist | 1 + > sysdeps/unix/sysv/linux/sh/be/libc.abilist | 1 + > sysdeps/unix/sysv/linux/sh/le/libc.abilist | 1 + > .../sysv/linux/sparc/sparc32/libc.abilist | 1 + > .../sysv/linux/sparc/sparc64/libc.abilist | 1 + > .../unix/sysv/linux/x86_64/64/libc.abilist | 1 + > .../unix/sysv/linux/x86_64/x32/libc.abilist | 1 + > 53 files changed, 1547 insertions(+), 1340 deletions(-) > create mode 100644 malloc/malloc-check.c > create mode 100644 malloc/malloc-compathooks.c > create mode 100644 malloc/mcheck-hooks.c > create mode 100644 malloc/mtrace-hooks.c >
On 7/3/21 12:35 AM, Carlos O'Donell wrote: > Thanks for working through this. > > This is looking better but we need to discuss the mcheck status and > dropping some of those interfaces, perhaps just deprecating libmcheck.a > and moving the functionality into libc_malloc_debug.so. See further review. > > I think we're almost done with a v4 review if we agree on direction. Thanks, I'm kinda emotional about malloc-check since it's bailed me out a number of times when valgrind/asan couldn't, but I suppose moving it along with the other debugging features to a different DSO isn't that bad. I'll keep them mostly weaned off the hooks but also move them to the DSO. Since it's in the DSO, mcheck can continue using the __malloc_initialize_hook to run the first mcheck() and we don't need a new ABI. I'll commit the first 4 patches (since they're source shuffling anyway) and make a v5 from the rest. Can I take till about Monday night to post the next set? I think it would be a pretty big security gain for us to finally move the debugging hooks out of the main library for 2.34. Thanks, Siddhesh
On 7/3/21 12:45 AM, Siddhesh Poyarekar via Libc-alpha wrote:
> I'll commit the first 4 patches (since they're source shuffling anyway)
I only pushed the first 3. I'll need the 4th one to make the next
patchset cleaner now that we don't want any trace of debugging in the
main malloc.
Siddhesh
This patchset removes the malloc hooks __malloc_hook, __free_hook, __realloc_hook and __memalign_hook from the API and leaves compatibility symbols so that existing applications can continue to link to them. The reading and execution of the hooks has been moved to a DSO libmalloc_compathooks.so, which can be preloaded for applications that need it. By default these hooks no longer have any effect in the library. Further, the __morecore, __morecore_after_hook and __default_morecore hooks have also been moved to compat symbols and removed from the API. Existing applications will continue to link to them but they won't have any effect on malloc behaviour. To enable this, the MALLOC_CHECK_, mcheck() and mtrace() hooks have been weaned away from hooks. In the process, some mcheck() failures have been fixed and the overall behaviour is now simpler to follow. Finally, tr_break and mallwatch symbols have been deprecated and are not used anywhere. Users are advised to use gdb watchpoints and conditional breakpoints to debug malloc internals since they ought to provide equivalent functionality. Changes from v3: - Remove source file dependencies - Commit mcheck tests Changes from v2: - Move hooks dependencies to malloc.o{,sS} Changes from v1: - Added makefile dependencies for the new hooks files - Fixed memset call in calloc debugging hooks - Added the tr_break deprecation patch and mcheck test patch to this series Siddhesh Poyarekar (10): Drop source dependencies on hooks.c and arena.c mtrace: Deprecate mallwatch and tr_break Move glibc.malloc.check implementation into its own file malloc: Move malloc hook references to hooks.c glibc.malloc.check: Wean away from malloc hooks mcheck: Wean away from malloc hooks mtrace: Wean away from malloc hooks Remove malloc hooks Remove __after_morecore_hook Remove __morecore and __default_morecore Makeconfig | 2 +- NEWS | 18 + include/malloc.h | 11 +- include/stdlib.h | 3 - malloc/Makefile | 23 +- malloc/Versions | 3 + malloc/arena.c | 26 +- malloc/hooks.c | 545 +++++++----------- malloc/malloc-check.c | 376 ++++++++++++ malloc/malloc-compathooks.c | 166 ++++++ malloc/malloc-internal.h | 6 + malloc/malloc.c | 235 ++++---- malloc/malloc.h | 27 - malloc/mcheck-hooks.c | 411 +++++++++++++ malloc/mcheck-init.c | 14 +- malloc/mcheck.c | 369 +----------- malloc/morecore.c | 15 +- malloc/mtrace-hooks.c | 137 +++++ malloc/mtrace.c | 276 +-------- manual/memory.texi | 191 +----- sysdeps/mach/hurd/i386/libc.abilist | 1 + sysdeps/unix/sysv/linux/aarch64/libc.abilist | 1 + sysdeps/unix/sysv/linux/alpha/libc.abilist | 1 + sysdeps/unix/sysv/linux/arc/libc.abilist | 1 + sysdeps/unix/sysv/linux/arm/be/libc.abilist | 1 + sysdeps/unix/sysv/linux/arm/le/libc.abilist | 1 + sysdeps/unix/sysv/linux/csky/libc.abilist | 1 + sysdeps/unix/sysv/linux/hppa/libc.abilist | 1 + sysdeps/unix/sysv/linux/i386/libc.abilist | 1 + sysdeps/unix/sysv/linux/ia64/libc.abilist | 1 + .../sysv/linux/m68k/coldfire/libc.abilist | 1 + .../unix/sysv/linux/m68k/m680x0/libc.abilist | 1 + .../sysv/linux/microblaze/be/libc.abilist | 1 + .../sysv/linux/microblaze/le/libc.abilist | 1 + .../sysv/linux/mips/mips32/fpu/libc.abilist | 1 + .../sysv/linux/mips/mips32/nofpu/libc.abilist | 1 + .../sysv/linux/mips/mips64/n32/libc.abilist | 1 + .../sysv/linux/mips/mips64/n64/libc.abilist | 1 + sysdeps/unix/sysv/linux/nios2/libc.abilist | 1 + .../linux/powerpc/powerpc32/fpu/libc.abilist | 1 + .../powerpc/powerpc32/nofpu/libc.abilist | 1 + .../linux/powerpc/powerpc64/be/libc.abilist | 1 + .../linux/powerpc/powerpc64/le/libc.abilist | 1 + .../unix/sysv/linux/riscv/rv32/libc.abilist | 1 + .../unix/sysv/linux/riscv/rv64/libc.abilist | 1 + .../unix/sysv/linux/s390/s390-32/libc.abilist | 1 + .../unix/sysv/linux/s390/s390-64/libc.abilist | 1 + sysdeps/unix/sysv/linux/sh/be/libc.abilist | 1 + sysdeps/unix/sysv/linux/sh/le/libc.abilist | 1 + .../sysv/linux/sparc/sparc32/libc.abilist | 1 + .../sysv/linux/sparc/sparc64/libc.abilist | 1 + .../unix/sysv/linux/x86_64/64/libc.abilist | 1 + .../unix/sysv/linux/x86_64/x32/libc.abilist | 1 + 53 files changed, 1547 insertions(+), 1340 deletions(-) create mode 100644 malloc/malloc-check.c create mode 100644 malloc/malloc-compathooks.c create mode 100644 malloc/mcheck-hooks.c create mode 100644 malloc/mtrace-hooks.c