Re: [PATCH v2] binfmt_elf: Move brk for static PIE even if ASLR disabled

[Kees Cook <kees@kernel.org>]

On Mon, May 12, 2025 at 04:17:12PM +0100, Ryan Roberts wrote:
> Hi Andrew,
> 
> 
> On 02/05/2025 11:01, Ryan Roberts wrote:
> > On 02/05/2025 01:18, Kees Cook wrote:
> >> In commit bbdc6076d2e5 ("binfmt_elf: move brk out of mmap when doing
> >> direct loader exec"), the brk was moved out of the mmap region when
> >> loading static PIE binaries (ET_DYN without INTERP). The common case
> >> for these binaries was testing new ELF loaders, so the brk needed to
> >> be away from mmap to avoid colliding with stack, future mmaps (of the
> >> loader-loaded binary), etc. But this was only done when ASLR was enabled,
> >> in an attempt to minimize changes to memory layouts.
> >>
> >> After adding support to respect alignment requirements for static PIE
> >> binaries in commit 3545deff0ec7 ("binfmt_elf: Honor PT_LOAD alignment
> >> for static PIE"), it became possible to have a large gap after the
> >> final PT_LOAD segment and the top of the mmap region. This means that
> >> future mmap allocations might go after the last PT_LOAD segment (where
> >> brk might be if ASLR was disabled) instead of before them (where they
> >> traditionally ended up).
> >>
> >> On arm64, running with ASLR disabled, Ubuntu 22.04's "ldconfig" binary,
> >> a static PIE, has alignment requirements that leaves a gap large enough
> >> after the last PT_LOAD segment to fit the vdso and vvar, but still leave
> >> enough space for the brk (which immediately follows the last PT_LOAD
> >> segment) to be allocated by the binary.
> >>
> >> fffff7f20000-fffff7fde000 r-xp 00000000 fe:02 8110426 /sbin/ldconfig.real
> >> fffff7fee000-fffff7ff5000 rw-p 000be000 fe:02 8110426 /sbin/ldconfig.real
> >> fffff7ff5000-fffff7ffa000 rw-p 00000000 00:00 0
> >> ***[brk will go here at fffff7ffa000]***
> >> fffff7ffc000-fffff7ffe000 r--p 00000000 00:00 0       [vvar]
> >> fffff7ffe000-fffff8000000 r-xp 00000000 00:00 0       [vdso]
> >> fffffffdf000-1000000000000 rw-p 00000000 00:00 0      [stack]
> >>
> >> After commit 0b3bc3354eb9 ("arm64: vdso: Switch to generic storage
> >> implementation"), the arm64 vvar grew slightly, and suddenly the brk
> >> collided with the allocation.
> >>
> >> fffff7f20000-fffff7fde000 r-xp 00000000 fe:02 8110426 /sbin/ldconfig.real
> >> fffff7fee000-fffff7ff5000 rw-p 000be000 fe:02 8110426 /sbin/ldconfig.real
> >> fffff7ff5000-fffff7ffa000 rw-p 00000000 00:00 0
> >> ***[oops, no room any more, vvar is at fffff7ffa000!]***
> >> fffff7ffa000-fffff7ffe000 r--p 00000000 00:00 0       [vvar]
> >> fffff7ffe000-fffff8000000 r-xp 00000000 00:00 0       [vdso]
> >> fffffffdf000-1000000000000 rw-p 00000000 00:00 0      [stack]
> 
> This change is fixing a pretty serious bug that appeared in v6.15-rc1 so I was
> hoping it would make it into the v6.15 final release. I'm guessing mm is the
> correct route in? But I don't currently see this in linus's tree or in any of
> your mm- staging branches. Is there still time to get this in?

I'll be sending it to Linus this week. I've been letting it bake in
-next for a while just to see if anything shakes out.

-- 
Kees Cook