Re: [PATCH v8 03/10] Remove __morecore and __default_morecore

[Siddhesh Poyarekar <siddhesh@gotplt.org>]

On 7/14/21 6:24 PM, Guillaume Morin wrote:
> On 14 Jul 12:31, Siddhesh Poyarekar wrote:
> I replied on the bug report becuase I had not seen this message.
> 
> But basically, this breaks https://github.com/libhugetlbfs/libhugetlbfs/
> which is old (started in 2006) and commonly used library.  The library
> is plugging its own morecore() implementation to use hugetlb pages to
> back the malloc heap.

Thanks, AFAICT, libhugetlbfs only uses __morecore and not the rest of 
the interfaces.  The thing is, __morecore and friends have been 
deprecated for a year and building anything with __morecore ought to 
give the deprecation warning.

> This is definitely not a win for all libhugetlbfs users. We have no
> equivalent solution. You're asking to find us an entire new malloc
> implementation if there is one (or write one?). We have no way of keep
> using glibc's malloc and hugetlb. I am not even sure there exists
> an equivalent replacement.
> 
> I understand there is some security concern about malloc hooks but then
> why not allow morecore() substitution with a properly
> documented/supported interface? Most users already LD_PRELOAD
> libhugetlbfs so that would be an easy fix.

I don't think we would like to export a different interface to do the 
same thing because it still doesn't allow us to clamp down further on 
the system malloc implementation.  Besides, the morecore hooking 
interface is severely limited because it only affects the main arena. 
Non-main arenas have their own allocation techniques and it doesn't 
really make sense to have an interface to control just the main arena.

> You're claiming that it's subtly broken. I'd like to understand how and
> why this can't be fixed. Afaik people have been using libhugetlbfs's
> morecore() in production for 15+ years without any issue.
> 
> The only issue I am aware of is the one I reported about 5 years ago
> (along with a reproducer and a patch). This problem can only be reached
> if trimming is enabled in the morecore implementation (it's disabled in
> libhugetlbfs for this very reason).

There is only the one known issue that you reported but that may well be 
because the interface isn't well tested.  There have been many changes 
to malloc over the years and I cannot say for certain that the idea that 
morecore could be anything other than brk has been consistently 
considered.  We don't have tests to verify that either.  Further, 
morecore limits its influence to the main arena and it is possible that 
future changes could break this in ways that we cannot anticipate today.

Basically this is technical debt we've accumulated from the days when 
malloc assumed single threaded programs and was incrementally hacked to 
add in multi-threaded support.  We'd like to clean this up.

The malloc code in glibc is quite general purpose and has little 
dependency on libc internals.  It may well be possible for you to copy 
the source files over into libhugetlbfs to implement the interposer 
within libhugetlbfs that does exactly what you need.  In fact, that may 
even allow you to extend hugetlbfs support to non-main arenas.

Siddhesh