linux-kernel - Re: [PATCH] binfmt_elf: Do not move brk for INTERP-less ET

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

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <cfdb3b68100025288177da8a963bc909@kojedz.in>
Date:   Thu, 26 Sep 2019 20:26:12 +0200
From:   Richard Kojedzinszky <richard@...edz.in>
To:     Kees Cook <keescook@...omium.org>
Cc:     Andrew Morton <akpm@...ux-foundation.org>,
        linux-kernel@...r.kernel.org, Ali Saidi <alisaidi@...zon.com>,
        Guenter Roeck <linux@...ck-us.net>,
        Michal Hocko <mhocko@...e.com>,
        Matthew Wilcox <willy@...radead.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Jann Horn <jannh@...gle.com>,
        Linus Torvalds <torvalds@...ux-foundation.org>
Subject: Re: [PATCH] binfmt_elf: Do not move brk for INTERP-less ET_EXEC

Hi Kees,

Thanks for the quick patch. It seems my binaries start up well, and work 
as expected, as before.

Thanks again for the quick response.

Regards,
Richard Kojedzinszky

2019-09-26 19:15 időpontban Kees Cook ezt írta:
> When brk was moved for binaries without an interpreter, it should have
> been limited to ET_DYN only. In other words, the special case was an
> ET_DYN that lacks an INTERP, not just an executable that lacks INTERP.
> The bug manifested for giant static executables, where the brk would 
> end
> up in the middle of the text area on 32-bit architectures.
> 
> Reported-by: Richard Kojedzinszky <richard@...edz.in>
> Fixes: bbdc6076d2e5 ("binfmt_elf: move brk out of mmap when doing
> direct loader exec")
> Cc: stable@...r.kernel.org
> Signed-off-by: Kees Cook <keescook@...omium.org>
> ---
> Richard, are you able to test this? I'm able to run the gitea binary
> with this change, and my INTERP-less ET_DYN tests (from the original
> bug) continue to pass as well.
> ---
>  fs/binfmt_elf.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
> index cec3b4146440..ad4c6b1d5074 100644
> --- a/fs/binfmt_elf.c
> +++ b/fs/binfmt_elf.c
> @@ -1121,7 +1121,8 @@ static int load_elf_binary(struct linux_binprm 
> *bprm)
>  		 * (since it grows up, and may collide early with the stack
>  		 * growing down), and into the unused ELF_ET_DYN_BASE region.
>  		 */
> -		if (IS_ENABLED(CONFIG_ARCH_HAS_ELF_RANDOMIZE) && !interpreter)
> +		if (IS_ENABLED(CONFIG_ARCH_HAS_ELF_RANDOMIZE) &&
> +		    loc->elf_ex.e_type == ET_DYN && !interpreter)
>  			current->mm->brk = current->mm->start_brk =
>  				ELF_ET_DYN_BASE;
> 
> --
> 2.17.1