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: Windows password security audit tool. GUI, reports in PDF.

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

Date:   Fri, 27 Sep 2019 08:16:53 +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,

Thanks for the fix, again. I am not familiar with the code here, I 
suspect the intent was to fix or improve something. That happens 
sometimes, that things get broken. I am happy that I could report it, 
and it got fixed quickly.

Thanks again,
Richard


2019-09-26 20:37 időpontban Kees Cook ezt írta:
> On Thu, Sep 26, 2019 at 08:26:12PM +0200, Richard Kojedzinszky wrote:
>> Thanks for the quick patch. It seems my binaries start up well, and 
>> work as
>> expected, as before.
>> 
>> Thanks again for the quick response.
> 
> Awesome; thanks for the testing (and sorry for the breakage)! :)
> 
> -Kees
> 
>> 
>> 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