Message-ID: <56655ac1-a650-43d8-8080-a03c250474d3@arm.com>
Date: Tue, 13 May 2025 10:19:40 +0100
From: Ryan Roberts <ryan.roberts@....com>
To: Kees Cook <kees@...nel.org>
Cc: Al Viro <viro@...iv.linux.org.uk>, Christian Brauner
 <brauner@...nel.org>, Jan Kara <jack@...e.cz>,
 Ali Saidi <alisaidi@...zon.com>, Andrew Morton <akpm@...ux-foundation.org>,
 linux-kernel@...r.kernel.org, linux-fsdevel@...r.kernel.org,
 linux-mm@...ck.org, linux-hardening@...r.kernel.org
Subject: Re: [PATCH v2] binfmt_elf: Move brk for static PIE even if ASLR
 disabled

On 12/05/2025 21:40, Kees Cook wrote:
> 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.

Sorry, Kees - my bad! For some reason, I thought this would go via the mm tree.
Thanks again for jumping on this.

Powered by blists - more mailing lists