| 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: <20160308045010.GA2481@x1.redhat.com>
Date: Tue, 8 Mar 2016 12:50:10 +0800
From: Baoquan He <bhe@...hat.com>
To: Kees Cook <keescook@...omium.org>
Cc: LKML <linux-kernel@...r.kernel.org>,
Yinghai Lu <yinghai@...nel.org>,
"H. Peter Anvin" <hpa@...or.com>, Vivek Goyal <vgoyal@...hat.com>,
Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
Andy Lutomirski <luto@...nel.org>, lasse.collin@...aani.org,
Andrew Morton <akpm@...ux-foundation.org>,
Dave Young <dyoung@...hat.com>
Subject: Re: [PATCH v3 02/19] x86, boot: Move compressed kernel to end of
buffer before decompressing
Hi Kees,
Many thanks for your review and suggestions!
On 03/07/16 at 02:35pm, Kees Cook wrote:
> > Current code uses extract_offset to decide the copied ZO position, it
> > put ZO starting from extract_offset. When INIT_SIZE is bigger than
> > VO_INIT_SIZE copied ZO occupies space from extract_offset to the end
> > of decompressing buffer, just like below.
> >
> > 0 extract_offset init_size
> > |-----------|---------------|------------------------|---------|
> > | | | |
> > VO__text startup_32 of ZO VO__end ZO__end
> >
> > However when INIT_SIZE is euqal to VO_INIT_SIZE there's still space left
>
> typo: "equal"
Yes, will change it in next post.
>
> > from end of ZO to the end of decompressing buffer, like below.
> >
> > 0 extract_offset init_size
> > |-----------|---------------|------------------------|---------|
> > | | | |
> > VO__text startup_32 of ZO ZO__end VO__end
> >
> > It's better to always put ZO right in the end of decompressing buffer.
> > Anyway, who doesn't like it to be better.
>
> It looks to me like it's just cleaner to always have ZO__end ==
> VO__end? Is that the motivation for moving it? What problem is solved
> with this change?
As you can see there are 2 cases. The first case is depicted in the 1st
graph. In that case ZO__end is after VO__end. In the 2nd case, yes, we
move ZO__end to make that happen ZO__end == VO__end. In fact this patch
is not trying to make them equal, what it is trying to do is adjusting
the behaviour. We always put ZO just in the end by getting init_size
which is the deompressing buffer and ZO_INIT_SIZE, not like before we
just put ZO starting from extract_offset. We change a different way to
get the starting position of ZO.
>
> -Kees
>
> >
> > So in this patch add BP_init_size into asm-offsets.c to make it visible
> > to asm modules. And find the starting position of copied ZO by init_size
> > and ZO run size, namely (ZO__end - startup_32) when move ZO.
> >
> > Signed-off-by: Yinghai Lu <yinghai@...nel.org>
> > ---
> > v2->v3:
> > Adjust the patch log.
> >
> > arch/x86/boot/compressed/head_32.S | 11 +++++++++--
> > arch/x86/boot/compressed/head_64.S | 8 ++++++--
> > arch/x86/boot/compressed/mkpiggy.c | 3 ---
> > arch/x86/boot/compressed/vmlinux.lds.S | 1 +
> > arch/x86/kernel/asm-offsets.c | 1 +
> > arch/x86/kernel/vmlinux.lds.S | 1 +
> > 6 files changed, 18 insertions(+), 7 deletions(-)
> >
> > diff --git a/arch/x86/boot/compressed/head_32.S b/arch/x86/boot/compressed/head_32.S
> > index 8ef964d..0c140f9 100644
> > --- a/arch/x86/boot/compressed/head_32.S
> > +++ b/arch/x86/boot/compressed/head_32.S
> > @@ -148,7 +148,9 @@ preferred_addr:
> > 1:
> >
> > /* Target address to relocate to for decompression */
> > - addl $z_extract_offset, %ebx
> > + movl BP_init_size(%esi), %eax
> > + subl $_end, %eax
> > + addl %eax, %ebx
> >
> > /* Set up the stack */
> > leal boot_stack_end(%ebx), %esp
> > @@ -210,8 +212,13 @@ relocated:
> > /* push arguments for decompress_kernel: */
> > pushl $z_run_size /* size of kernel with .bss and .brk */
> > pushl $z_output_len /* decompressed length, end of relocs */
> > - leal z_extract_offset_negative(%ebx), %ebp
> > +
> > + movl BP_init_size(%esi), %eax
> > + subl $_end, %eax
> > + movl %ebx, %ebp
> > + subl %eax, %ebp
> > pushl %ebp /* output address */
> > +
> > pushl $z_input_len /* input_len */
> > leal input_data(%ebx), %eax
> > pushl %eax /* input_data */
> > diff --git a/arch/x86/boot/compressed/head_64.S b/arch/x86/boot/compressed/head_64.S
> > index b0c0d16..67dd8d3 100644
> > --- a/arch/x86/boot/compressed/head_64.S
> > +++ b/arch/x86/boot/compressed/head_64.S
> > @@ -102,7 +102,9 @@ ENTRY(startup_32)
> > 1:
> >
> > /* Target address to relocate to for decompression */
> > - addl $z_extract_offset, %ebx
> > + movl BP_init_size(%esi), %eax
> > + subl $_end, %eax
> > + addl %eax, %ebx
> >
> > /*
> > * Prepare for entering 64 bit mode
> > @@ -330,7 +332,9 @@ preferred_addr:
> > 1:
> >
> > /* Target address to relocate to for decompression */
> > - leaq z_extract_offset(%rbp), %rbx
> > + movl BP_init_size(%rsi), %ebx
> > + subl $_end, %ebx
> > + addq %rbp, %rbx
> >
> > /* Set up the stack */
> > leaq boot_stack_end(%rbx), %rsp
> > diff --git a/arch/x86/boot/compressed/mkpiggy.c b/arch/x86/boot/compressed/mkpiggy.c
> > index d8222f2..b980046 100644
> > --- a/arch/x86/boot/compressed/mkpiggy.c
> > +++ b/arch/x86/boot/compressed/mkpiggy.c
> > @@ -85,9 +85,6 @@ int main(int argc, char *argv[])
> > printf("z_output_len = %lu\n", (unsigned long)olen);
> > printf(".globl z_extract_offset\n");
> > printf("z_extract_offset = 0x%lx\n", offs);
> > - /* z_extract_offset_negative allows simplification of head_32.S */
> > - printf(".globl z_extract_offset_negative\n");
> > - printf("z_extract_offset_negative = -0x%lx\n", offs);
> > printf(".globl z_run_size\n");
> > printf("z_run_size = %lu\n", run_size);
> >
> > diff --git a/arch/x86/boot/compressed/vmlinux.lds.S b/arch/x86/boot/compressed/vmlinux.lds.S
> > index 34d047c..e24e0a0 100644
> > --- a/arch/x86/boot/compressed/vmlinux.lds.S
> > +++ b/arch/x86/boot/compressed/vmlinux.lds.S
> > @@ -70,5 +70,6 @@ SECTIONS
> > _epgtable = . ;
> > }
> > #endif
> > + . = ALIGN(PAGE_SIZE); /* keep ZO size page aligned */
> > _end = .;
> > }
> > diff --git a/arch/x86/kernel/asm-offsets.c b/arch/x86/kernel/asm-offsets.c
> > index 84a7524..506de14 100644
> > --- a/arch/x86/kernel/asm-offsets.c
> > +++ b/arch/x86/kernel/asm-offsets.c
> > @@ -81,6 +81,7 @@ void common(void) {
> > OFFSET(BP_hardware_subarch, boot_params, hdr.hardware_subarch);
> > OFFSET(BP_version, boot_params, hdr.version);
> > OFFSET(BP_kernel_alignment, boot_params, hdr.kernel_alignment);
> > + OFFSET(BP_init_size, boot_params, hdr.init_size);
> > OFFSET(BP_pref_address, boot_params, hdr.pref_address);
> > OFFSET(BP_code32_start, boot_params, hdr.code32_start);
> >
> > diff --git a/arch/x86/kernel/vmlinux.lds.S b/arch/x86/kernel/vmlinux.lds.S
> > index 74e4bf1..217d5b1 100644
> > --- a/arch/x86/kernel/vmlinux.lds.S
> > +++ b/arch/x86/kernel/vmlinux.lds.S
> > @@ -325,6 +325,7 @@ SECTIONS
> > __brk_limit = .;
> > }
> >
> > + . = ALIGN(PAGE_SIZE); /* keep VO_INIT_SIZE page aligned */
> > _end = .;
> >
> > STABS_DEBUG
> > --
> > 2.5.0
> >
>
>
>
> --
> Kees Cook
> Chrome OS & Brillo Security
Powered by blists - more mailing lists