[RFC] Fix abort in malloc extent coalescing.

[dgr_semihalf.com]

Summary

Fix error in extent_try_coalesce_impl(), which could cause abort
to happen when trying to coalesce extents backwards. The error could
happen because of how extent_before_get() function works. This function
gets address of previous extent, by subtracting page size from current
extent address. If current extent is located at PAGE_SIZE offset, this
address resolved to 0x0000. An assertion in rtree_leaf_elm_lookup
then caused the running program to abort.

This problem was discovered when trying to build world on 32-bit
machines with ASLR and PIE enabled. The problem was present
on armv7 and i386 machines, however other 32-bit machines were
not tested, and the same problem is most likely seen there.
While this fixes one problem with buildworld on 32-bit platforms
with ASLR, the build still fails, however it happens much later
and due to lack of memory.

No such problem was observed on 64-bit platforms, however it seems
that it could also happen there, but the probability of such abort
happening is much lower due to significantly larger address space.

[kib]

I think this change is fine as is.

But isn't there a further problem then ? If sysctl security.bsd.map_at_zero is true, then in principle mmap could create a mapping at address zero, on malloc request. Then wouldn't we get a similar issue ?

[lwhsu]

Does it make sense also report to the upstream? https://github.com/jemalloc/jemalloc

[dgr_semihalf.com]

In D27025#603548, @lwhsu wrote:

Does it make sense also report to the upstream? https://github.com/jemalloc/jemalloc

I opened a Pull Request in upstream repository: https://github.com/jemalloc/jemalloc/pull/1973

In D27025#602992, @kib wrote:

I think this change is fine as is.

But isn't there a further problem then ? If sysctl security.bsd.map_at_zero is true, then in principle mmap could create a mapping at address zero, on malloc request. Then wouldn't we get a similar issue ?

Yes, in this case the assertion would fail anyway. However, allowing mapping at address zero is not a typical scenario and there are more NULL checks that will fail when mmap returns zero.

[mw]

Hi,

In jemalloc upstream repo based on this patch another PR was merged, that applies on top of tree (https://github.com/jemalloc/jemalloc/pull/2003).

What we have in contrib/jemalloc is not aligned to jemalloc upstream and IMO we should merge this commit as-is. Any objections @kib, @lwhsu ?

Best regards,
Marcin

[kib]

I do not have objections.