Re: [musl] mallocng switchover - opportunity to test

[Rich Felker <dalias@libc.org>]

On Tue, Jun 09, 2020 at 01:09:14PM +0200, Szabolcs Nagy wrote:
> * Rich Felker <dalias@libc.org> [2020-06-08 23:50:10 -0400]:
> > This produces a near-fully-integrated malloc, including support for
> > reclaim_gaps donation from ldso. The only functionality missing, which
> > I expect to flesh out before actual import, is handling of the case of
> > incomplete malloc replacement by interposition (__malloc_replaced!=0).
> 
> i would actually prefer if we didn't check for __malloc_replaced
> in aligned alloc, because i think it does not provide significant
> safety, but it prevents the simple RTLD_NEXT wrappers which are
> commonly used for simple malloc debugging/tracing/etc (and while
> unsafe in general depending on what libc api calls they make,
> they likely work in practice).
> 
> (the check does not provide safety because existing interposers
> written for glibc likely work with musl too without issues:
> the only problem is if musl uses aligned alloc somewhere where
> glibc does not so an interposer may work on glibc without
> interposing aligned alloc but not on musl. for newly written
> interposers we just need to document the api contract.)

I'm not sure about this, and how it interacts with our definition of
posix_memalign and memalign in terms of aligned_alloc.

> fyi, i looked at enabling AArch64 MTE in malloc ng, with the
> current linux syscall abi patches i saw the following issues:
> 
> - brk is not usable (this might change in the final linux patches
>   but it's nice that we can turn brk usage off in malloc-ng)

mallocng does not use brk for allocations, only (at most) for
metadata areas. And the metadata areas are supposed to be in a place
that's hard to hit via memory errors. So in some sense it may be
acceptable to just omit MTE for them. Disabling brk puts them in a
more predictable place (mmap zone), but being able to protect them
with MTE (with a tag not used for anything else) should give much
greated protection anyway.

> - reclaim_gaps is not usable (in linux, file content may be in
>   memory that does not support mte, even for private CoW maps,
>   this can be changed in principle but to use reclaim_gaps elf
>   changes will be needed anyway so a loader knows it has to
>   use PROT_MTE and there is no elf abi design for that yet)

Sometimes (often) the gaps will be in bss outside p_filesz, so those
should be usable even without fixing this on the kernel side. But
indeed it would be best to just have it always work right.

Probably __malloc_donate should, if built for MTE, attempt to mprotect
with PROT_MTE and decline to use the memory if it can't.

> - meta data at the end of an allocation slot cannot share the
>   same tagging granule (16byte) with user allocation (otherwise
>   accessing it would become akward due to possible concurrent
>   tag changes, even if we don't care about protecting that meta
>   data with mte) hence 16 bytes has to be reserved for meta data,
>   this impacts the overhead of small allocations (the required
>   code change is small given it already uses 16byte units, but
>   since the layout of allocations is affected this is probably
>   the most intrusive change mte requires).

Yes, the hard-coded 4 needs to be made into a macro. Sorry I didn't gt
a chance to review/merge your patch for that when you posted it; I had
too many other changes in process.

> - madvise MADV_FREE means naive tagging of internal/freed memory
>   with a reseved internal tag does not work: internal pointers
>   cannot access memory after they are zeroed by the kernel.
>   this can be fixed in various ways i haven't decided what's
>   best yet. enabling mte will cause various regressions and
>   different behaviour (e.g. because all pages are written to on
>   malloc, calloc, realloc, free) this will be one of them.

Can you clarify what goes wrong here? There shouldn't be access to
memory that's been freed except at time of enframe, and enframe should
be able to set a new tag at the same time it performs the access.

> if support for this is interesting i can work on patches
> that can be upstreamed (e.g. macros conditional on mte
> support etc)

There is interest, at least from me, and I hope we can also influence
improvement of things on the ELF and kernel sides.

I think it would be helpful to see patches even if they're a total
hack, before you work on polishing them for inclusion, since I might
have ideas how to change things to make the patches simpler and make
MTE less invasive.

Rich