| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <202504251158.D3D342410@keescook>
Date: Fri, 25 Apr 2025 12:56:21 -0700
From: Kees Cook <kees@...nel.org>
To: Catalin Marinas <catalin.marinas@....com>
Cc: Ryan Roberts <ryan.roberts@....com>,
Thomas Weißschuh <thomas.weissschuh@...utronix.de>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
Linux-MM <linux-mm@...ck.org>,
"linux-arm-kernel@...ts.infradead.org" <linux-arm-kernel@...ts.infradead.org>
Subject: Re: BUG: vdso changes expose elf mapping issue
On Fri, Apr 25, 2025 at 07:37:38PM +0100, Catalin Marinas wrote:
> On Fri, Apr 25, 2025 at 01:41:31PM +0100, Ryan Roberts wrote:
> > ldconfig is a statically linked, PIE executable. The kernel treats this as an
> > interpreter and therefore does not map it into low memory but instead maps it
> > into high memory using mmap() (mmap is top-down on arm64). Once it's mapped,
> > vvar/vdso gets mapped and fills the hole right at the top that is left due to
> > ldconfig's alignment requirements. Before the above change, there were 2 pages
> > free between the end of the data segment and vvar; this was enough for ldconfig
> > to get it's required memory with brk(). But after the change there is no space:
> >
> > Before:
> > fffff7f20000-fffff7fde000 r-xp 00000000 fe:02 8110426 /home/ubuntu/glibc-2.35/build/elf/ldconfig
> > fffff7fee000-fffff7ff5000 rw-p 000be000 fe:02 8110426 /home/ubuntu/glibc-2.35/build/elf/ldconfig
> > fffff7ff5000-fffff7ffa000 rw-p 00000000 00:00 0
> > 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:
> > fffff7f20000-fffff7fde000 r-xp 00000000 fe:02 8110426 /home/ubuntu/glibc-2.35/build/elf/ldconfig
> > fffff7fee000-fffff7ff5000 rw-p 000be000 fe:02 8110426 /home/ubuntu/glibc-2.35/build/elf/ldconfig
> > fffff7ff5000-fffff7ffa000 rw-p 00000000 00:00 0
> > 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]
>
> It does look like we've just been lucky so far. An ELF file requiring a
> slightly larger brk (by two pages), it could fail. FWIW, briefly after
> commit 9630f0d60fec ("fs/binfmt_elf: use PT_LOAD p_align values for
> static PIE"), we got:
>
> Start Addr End Addr Size Offset Perms objfile
> 0xaaaaaaaa0000 0xaaaaaab5d000 0xbd000 0x0 r-xp /usr/sbin/ldconfig
> 0xaaaaaab6b000 0xaaaaaab73000 0x8000 0xcb000 rw-p /usr/sbin/ldconfig
> 0xaaaaaab73000 0xaaaaaab78000 0x5000 0x0 rw-p [heap]
> 0xfffff7ffd000 0xfffff7fff000 0x2000 0x0 r--p [vvar]
> 0xfffff7fff000 0xfffff8000000 0x1000 0x0 r-xp [vdso]
> 0xfffffffdf000 0x1000000000000 0x21000 0x0 rw-p [stack]
>
> This looks like a better layout to me when you load an ET_DYN file
> without !PT_INTERP.
The trouble is that !PT_INTERP must be loaded out of the way of the
binary it may load, so it cannot be loaded low.
> When the commit was reverted by aeb7923733d1 ("revert "fs/binfmt_elf:
> use PT_LOAD p_align values for static PIE""), we went back to:
>
> Start Addr End Addr Size Offset Perms objfile
> 0xfffff7f28000 0xfffff7fe5000 0xbd000 0x0 r-xp /usr/sbin/ldconfig
> 0xfffff7ff0000 0xfffff7ff2000 0x2000 0x0 r--p [vvar]
> 0xfffff7ff2000 0xfffff7ff3000 0x1000 0x0 r-xp [vdso]
> 0xfffff7ff3000 0xfffff7ffb000 0x8000 0xcb000 rw-p /usr/sbin/ldconfig
> 0xfffff7ffb000 0xfffff8000000 0x5000 0x0 rw-p [heap]
> 0xfffffffdf000 0x1000000000000 0x21000 0x0 rw-p [stack]
The revert was because, among various additional problems, that this low
load would collide with things. The static PIE alignment was finally
fixed with commit 3545deff0ec7 ("binfmt_elf: Honor PT_LOAD alignment
for static PIE")
The ultimate brk location is determined near the end of load_elf_binary()
(see the code surrounding the comment "Otherwise leave a gap").
> With 6.15-rc3 my layout looks like Ryan's but in 5.18 above, the vdso is
> small enough and it's squeezed between the two ldconfig sections.
I think there are two surprises:
- For loaders (ET_DYN without PT_INTERP, which is also "static PIE") the
brk location is being moved to ELF_ET_DYN_BASE ... *but only when ASLR
is enabled*. I think exclusion is the primary bug, with its origin
in commit bbdc6076d2e5 ("binfmt_elf: move brk out of mmap when doing
direct loader exec"). I failed to explain my rationale at the time
to have it only happen under ASLR, but I think I was trying to be
conservative and not change things too much.
- vdso can get loaded into _gaps_ in the ELF. I think this is asking for
trouble, but technically should be okay since neither can grow. But I
never like seeing immediately adjacent unrelated mappings, since we
always end up with bugs (see things like commit 2a5eb9995528
("binfmt_elf: Leave a gap between .bss and brk").
For fixing the former, the below change might work (totally untested yet,
I just wanted to reply with my thoughts as I start testing this). Pardon
the goofy code style, I wanted a minimal diff here:
diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
index 7e2afe3220f7..9290a29ede28 100644
--- a/fs/binfmt_elf.c
+++ b/fs/binfmt_elf.c
@@ -1284,7 +1284,7 @@ static int load_elf_binary(struct linux_binprm *bprm)
mm->end_data = end_data;
mm->start_stack = bprm->p;
- if ((current->flags & PF_RANDOMIZE) && (snapshot_randomize_va_space > 1)) {
+ {
/*
* For architectures with ELF randomization, when executing
* a loader directly (i.e. no interpreter listed in ELF
@@ -1299,7 +1299,9 @@ static int load_elf_binary(struct linux_binprm *bprm)
/* Otherwise leave a gap between .bss and brk. */
mm->brk = mm->start_brk = mm->brk + PAGE_SIZE;
}
+ }
+ if ((current->flags & PF_RANDOMIZE) && (snapshot_randomize_va_space > 1)) {
mm->brk = mm->start_brk = arch_randomize_brk(mm);
#ifdef compat_brk_randomized
current->brk_randomized = 1;
> > Note that this issue only occurs with ASLR disabled. When ASLR is enabled, the
> > brk region is setup in the low memory region that would normally be used by
> > primary executable.
Out of curiosity, why are you running without ASLR?
Thanks for the report! I'll continue testing the above fix. Just for
making sure I am able to exactly reproduce your issue, this is on
a regular arm64 install of Ubuntu 22.04?
-Kees
--
Kees Cook
Powered by blists - more mailing lists