* [PATCH] binfmt_elf: Do not move brk for INTERP-less ET_EXEC
@ 2019-09-26 17:15 Kees Cook
2019-09-26 18:26 ` Richard Kojedzinszky
0 siblings, 1 reply; 4+ messages in thread
From: Kees Cook @ 2019-09-26 17:15 UTC (permalink / raw)
To: Andrew Morton
Cc: Richard Kojedzinszky, linux-kernel, Ali Saidi, Guenter Roeck,
Michal Hocko, Matthew Wilcox, Thomas Gleixner, Jann Horn,
Linus Torvalds
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@kojedz.in>
Fixes: bbdc6076d2e5 ("binfmt_elf: move brk out of mmap when doing direct loader exec")
Cc: stable@vger.kernel.org
Signed-off-by: Kees Cook <keescook@chromium.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
--
Kees Cook
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] binfmt_elf: Do not move brk for INTERP-less ET_EXEC
2019-09-26 17:15 [PATCH] binfmt_elf: Do not move brk for INTERP-less ET_EXEC Kees Cook
@ 2019-09-26 18:26 ` Richard Kojedzinszky
2019-09-26 18:37 ` Kees Cook
0 siblings, 1 reply; 4+ messages in thread
From: Richard Kojedzinszky @ 2019-09-26 18:26 UTC (permalink / raw)
To: Kees Cook
Cc: Andrew Morton, linux-kernel, Ali Saidi, Guenter Roeck,
Michal Hocko, Matthew Wilcox, Thomas Gleixner, Jann Horn,
Linus Torvalds
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@kojedz.in>
> Fixes: bbdc6076d2e5 ("binfmt_elf: move brk out of mmap when doing
> direct loader exec")
> Cc: stable@vger.kernel.org
> Signed-off-by: Kees Cook <keescook@chromium.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
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] binfmt_elf: Do not move brk for INTERP-less ET_EXEC
2019-09-26 18:26 ` Richard Kojedzinszky
@ 2019-09-26 18:37 ` Kees Cook
2019-09-27 6:16 ` Richard Kojedzinszky
0 siblings, 1 reply; 4+ messages in thread
From: Kees Cook @ 2019-09-26 18:37 UTC (permalink / raw)
To: Richard Kojedzinszky
Cc: Andrew Morton, linux-kernel, Ali Saidi, Guenter Roeck,
Michal Hocko, Matthew Wilcox, Thomas Gleixner, Jann Horn,
Linus Torvalds
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@kojedz.in>
> > Fixes: bbdc6076d2e5 ("binfmt_elf: move brk out of mmap when doing
> > direct loader exec")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Kees Cook <keescook@chromium.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
--
Kees Cook
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] binfmt_elf: Do not move brk for INTERP-less ET_EXEC
2019-09-26 18:37 ` Kees Cook
@ 2019-09-27 6:16 ` Richard Kojedzinszky
0 siblings, 0 replies; 4+ messages in thread
From: Richard Kojedzinszky @ 2019-09-27 6:16 UTC (permalink / raw)
To: Kees Cook
Cc: Andrew Morton, linux-kernel, Ali Saidi, Guenter Roeck,
Michal Hocko, Matthew Wilcox, Thomas Gleixner, Jann Horn,
Linus Torvalds
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@kojedz.in>
>> > Fixes: bbdc6076d2e5 ("binfmt_elf: move brk out of mmap when doing
>> > direct loader exec")
>> > Cc: stable@vger.kernel.org
>> > Signed-off-by: Kees Cook <keescook@chromium.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
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-09-27 6:16 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-26 17:15 [PATCH] binfmt_elf: Do not move brk for INTERP-less ET_EXEC Kees Cook
2019-09-26 18:26 ` Richard Kojedzinszky
2019-09-26 18:37 ` Kees Cook
2019-09-27 6:16 ` Richard Kojedzinszky
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).