| 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: <CAMj1kXE2D_iWGQODd2t=9-URE1T2N4+hgrTn7OQa-=ma=kncvg@mail.gmail.com>
Date: Wed, 15 Jul 2020 12:03:07 +0300
From: Ard Biesheuvel <ardb@...nel.org>
To: Sedat Dilek <sedat.dilek@...il.com>
Cc: Arvind Sankar <nivedita@...m.mit.edu>,
Thomas Gleixner <tglx@...utronix.de>,
Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
"H. Peter Anvin" <hpa@...or.com>, X86 ML <x86@...nel.org>,
Nick Desaulniers <ndesaulniers@...gle.com>,
Fangrui Song <maskray@...gle.com>,
Dmitry Golovin <dima@...ovin.in>,
Clang-Built-Linux ML <clang-built-linux@...glegroups.com>,
Masahiro Yamada <masahiroy@...nel.org>,
Daniel Kiper <daniel.kiper@...cle.com>,
Kees Cook <keescook@...omium.org>,
Nathan Chancellor <natechancellor@...il.com>,
Arnd Bergmann <arnd@...db.de>,
"H . J . Lu" <hjl@...rceware.org>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v5 6/7] x86/boot: Remove run-time relocations from head_{32,64}.S
On Wed, 15 Jul 2020 at 11:58, Sedat Dilek <sedat.dilek@...il.com> wrote:
>
> On Wed, Jul 15, 2020 at 2:41 AM Arvind Sankar <nivedita@...m.mit.edu> wrote:
> >
> > The BFD linker generates run-time relocations for z_input_len and
> > z_output_len, even though they are absolute symbols.
> >
> > This is fixed for binutils-2.35 [1]. Work around this for earlier
> > versions by defining two variables input_len and output_len in addition
> > to the symbols, and use them via position-independent references.
> >
> > This eliminates the last two run-time relocations in the head code and
> > allows us to drop the -z noreloc-overflow flag to the linker.
> >
> > Move the -pie and --no-dynamic-linker LDFLAGS to LDFLAGS_vmlinux instead
> > of KBUILD_LDFLAGS. There shouldn't be anything else getting linked, but
> > this is the more logical location for these flags, and modversions might
> > call the linker if an EXPORT_SYMBOL is left over accidentally in one of
> > the decompressors.
> >
>
> Tested-by: Sedat Dilek <sedat.dilek@...il.com>
> Reported-by: Sedat Dilek <sedat.dilek@...il.com>
>
> Reported breakage with LLD in previous patchset version.
>
Please drop the bogus reported-bys. The patch does *not* address an
issue you reported, so recording this in the commit log would be
incorrect.
Your review and testing is appreciated, and resulted in substantial
improvements. So feel free to give your reviewed-by in addition to
your tested-by. But reported-by is inappropriate here.
>
> > [1] https://sourceware.org/bugzilla/show_bug.cgi?id=25754
> >
> > Reviewed-by: Kees Cook <keescook@...omium.org>
> > Reviewed-by: Ard Biesheuvel <ardb@...nel.org>
> > Reviewed-by: Fangrui Song <maskray@...gle.com>
> > Signed-off-by: Arvind Sankar <nivedita@...m.mit.edu>
> > ---
> > arch/x86/boot/compressed/Makefile | 12 ++----------
> > arch/x86/boot/compressed/head_32.S | 17 ++++++++---------
> > arch/x86/boot/compressed/head_64.S | 4 ++--
> > arch/x86/boot/compressed/mkpiggy.c | 6 ++++++
> > 4 files changed, 18 insertions(+), 21 deletions(-)
> >
> > diff --git a/arch/x86/boot/compressed/Makefile b/arch/x86/boot/compressed/Makefile
> > index c829d874dcac..ae2c0dc98a6a 100644
> > --- a/arch/x86/boot/compressed/Makefile
> > +++ b/arch/x86/boot/compressed/Makefile
> > @@ -51,16 +51,8 @@ UBSAN_SANITIZE :=n
> > KBUILD_LDFLAGS := -m elf_$(UTS_MACHINE)
> > # Compressed kernel should be built as PIE since it may be loaded at any
> > # address by the bootloader.
> > -ifeq ($(CONFIG_X86_32),y)
> > -KBUILD_LDFLAGS += $(call ld-option, -pie) $(call ld-option, --no-dynamic-linker)
> > -else
> > -# To build 64-bit compressed kernel as PIE, we disable relocation
> > -# overflow check to avoid relocation overflow error with a new linker
> > -# command-line option, -z noreloc-overflow.
> > -KBUILD_LDFLAGS += $(shell $(LD) --help 2>&1 | grep -q "\-z noreloc-overflow" \
> > - && echo "-z noreloc-overflow -pie --no-dynamic-linker")
> > -endif
> > -LDFLAGS_vmlinux := -T
> > +LDFLAGS_vmlinux := $(call ld-option, -pie) $(call ld-option, --no-dynamic-linker)
> > +LDFLAGS_vmlinux += -T
> >
> > hostprogs := mkpiggy
> > HOST_EXTRACFLAGS += -I$(srctree)/tools/include
> > diff --git a/arch/x86/boot/compressed/head_32.S b/arch/x86/boot/compressed/head_32.S
> > index 8c1a4f5610f5..659fad53ca82 100644
> > --- a/arch/x86/boot/compressed/head_32.S
> > +++ b/arch/x86/boot/compressed/head_32.S
> > @@ -178,18 +178,17 @@ SYM_FUNC_START_LOCAL_NOALIGN(.Lrelocated)
> > /*
> > * Do the extraction, and jump to the new kernel..
> > */
> > - /* push arguments for extract_kernel: */
> > - pushl $z_output_len /* decompressed length, end of relocs */
> > + /* push arguments for extract_kernel: */
> >
> > - pushl %ebp /* output address */
> > -
> > - pushl $z_input_len /* input_len */
> > + pushl output_len@...OFF(%ebx) /* decompressed length, end of relocs */
> > + pushl %ebp /* output address */
> > + pushl input_len@...OFF(%ebx) /* input_len */
> > leal input_data@...OFF(%ebx), %eax
> > - pushl %eax /* input_data */
> > + pushl %eax /* input_data */
> > leal boot_heap@...OFF(%ebx), %eax
> > - pushl %eax /* heap area */
> > - pushl %esi /* real mode pointer */
> > - call extract_kernel /* returns kernel location in %eax */
> > + pushl %eax /* heap area */
> > + pushl %esi /* real mode pointer */
> > + call extract_kernel /* returns kernel location in %eax */
> > addl $24, %esp
> >
> > /*
> > diff --git a/arch/x86/boot/compressed/head_64.S b/arch/x86/boot/compressed/head_64.S
> > index 11429092c224..9e46729cf162 100644
> > --- a/arch/x86/boot/compressed/head_64.S
> > +++ b/arch/x86/boot/compressed/head_64.S
> > @@ -534,9 +534,9 @@ SYM_FUNC_START_LOCAL_NOALIGN(.Lrelocated)
> > movq %rsi, %rdi /* real mode address */
> > leaq boot_heap(%rip), %rsi /* malloc area for uncompression */
> > leaq input_data(%rip), %rdx /* input_data */
> > - movl $z_input_len, %ecx /* input_len */
> > + movl input_len(%rip), %ecx /* input_len */
> > movq %rbp, %r8 /* output target address */
> > - movl $z_output_len, %r9d /* decompressed length, end of relocs */
> > + movl output_len(%rip), %r9d /* decompressed length, end of relocs */
> > call extract_kernel /* returns kernel location in %rax */
> > popq %rsi
> >
> > diff --git a/arch/x86/boot/compressed/mkpiggy.c b/arch/x86/boot/compressed/mkpiggy.c
> > index 7e01248765b2..52aa56cdbacc 100644
> > --- a/arch/x86/boot/compressed/mkpiggy.c
> > +++ b/arch/x86/boot/compressed/mkpiggy.c
> > @@ -60,6 +60,12 @@ int main(int argc, char *argv[])
> > printf(".incbin \"%s\"\n", argv[1]);
> > printf("input_data_end:\n");
> >
> > + printf(".section \".rodata\",\"a\",@progbits\n");
> > + printf(".globl input_len\n");
> > + printf("input_len:\n\t.long %lu\n", ilen);
> > + printf(".globl output_len\n");
> > + printf("output_len:\n\t.long %lu\n", (unsigned long)olen);
> > +
> > retval = 0;
> > bail:
> > if (f)
> > --
> > 2.26.2
> >
Powered by blists - more mailing lists