From: Baoquan He <bhe@redhat.com>
To: Kees Cook <keescook@chromium.org>
Cc: LKML <linux-kernel@vger.kernel.org>,
Yinghai Lu <yinghai@kernel.org>, "H. Peter Anvin" <hpa@zytor.com>,
Vivek Goyal <vgoyal@redhat.com>, Ingo Molnar <mingo@redhat.com>,
Borislav Petkov <bp@alien8.de>, Andy Lutomirski <luto@kernel.org>,
lasse.collin@tukaani.org,
Andrew Morton <akpm@linux-foundation.org>,
Dave Young <dyoung@redhat.com>
Subject: Re: [PATCH v3 02/19] x86, boot: Move compressed kernel to end of buffer before decompressing
Date: Tue, 8 Mar 2016 12:50:10 +0800 [thread overview]
Message-ID: <20160308045010.GA2481@x1.redhat.com> (raw)
In-Reply-To: <CAGXu5jK7xR+hsX6-2g5gTo6_0-WEMv=y=oFXuKUjFvErbX+=xg@mail.gmail.com>
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@kernel.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
next prev parent reply other threads:[~2016-03-08 4:50 UTC|newest]
Thread overview: 51+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-03-04 16:24 [PATCH v3 00/19] x86, boot: kaslr cleanup and 64bit kaslr support Baoquan He
2016-03-04 16:24 ` [PATCH v3 01/19] x86, kaslr: Remove not needed parameter for choose_kernel_location Baoquan He
2016-03-07 22:28 ` Kees Cook
2016-03-04 16:25 ` [PATCH v3 02/19] x86, boot: Move compressed kernel to end of buffer before decompressing Baoquan He
2016-03-07 22:35 ` Kees Cook
2016-03-08 4:50 ` Baoquan He [this message]
2016-03-04 16:25 ` [PATCH v3 03/19] x86, boot: Move z_extract_offset calculation to header.S Baoquan He
2016-03-04 16:25 ` [PATCH v3 04/19] x86, kaskr: Update the description for decompressor worst case Baoquan He
2016-03-04 16:25 ` [PATCH v3 05/19] x86, boot: Fix run_size calculation Baoquan He
2016-03-07 23:10 ` Kees Cook
2016-03-08 4:57 ` Baoquan He
2016-03-08 18:05 ` Kees Cook
2016-03-04 16:25 ` [PATCH v3 06/19] x86, kaslr: Clean up useless code related to run_size Baoquan He
2016-03-07 23:12 ` Kees Cook
2016-03-08 5:00 ` Baoquan He
2016-03-04 16:25 ` [PATCH v3 07/19] x86, kaslr: Get correct max_addr for relocs pointer Baoquan He
2016-03-07 23:16 ` Kees Cook
2016-03-08 5:13 ` Baoquan He
2016-03-08 18:16 ` Kees Cook
2016-03-09 13:46 ` Baoquan He
2016-03-04 16:25 ` [PATCH v3 08/19] x86, kaslr: Consolidate mem_avoid array filling Baoquan He
2016-03-07 23:28 ` Kees Cook
2016-03-08 5:21 ` Baoquan He
2016-03-08 18:17 ` Kees Cook
2016-03-09 13:42 ` Baoquan He
2016-03-04 16:25 ` [PATCH v3 09/19] x86, boot: Split kernel_ident_mapping_init to another file Baoquan He
2016-03-04 16:25 ` [PATCH v3 10/19] x86, 64bit: Set ident_mapping for kaslr Baoquan He
2016-03-07 23:34 ` Kees Cook
2016-03-08 5:25 ` Baoquan He
2016-03-21 7:50 ` Baoquan He
2016-03-21 19:48 ` Kees Cook
2016-03-04 16:25 ` [PATCH v3 11/19] x86, boot: Add checking for memcpy Baoquan He
2016-03-07 23:36 ` Kees Cook
2016-03-08 5:30 ` Baoquan He
2016-03-04 16:25 ` [PATCH v3 12/19] x86, kaslr: Fix a bug that relocation can not be handled when kernel is loaded above 2G Baoquan He
2016-03-07 23:30 ` Kees Cook
2016-03-08 5:22 ` Baoquan He
2016-03-04 16:25 ` [PATCH v3 13/19] x86, kaslr: Introduce struct slot_area to manage randomization slot info Baoquan He
2016-03-04 16:25 ` [PATCH v3 14/19] x86, kaslr: Add two functions which will be used later Baoquan He
2016-03-04 16:25 ` [PATCH v3 15/19] x86, kaslr: Introduce fetch_random_virt_offset to randomize the kernel text mapping address Baoquan He
2016-03-04 16:25 ` [PATCH v3 16/19] x86, kaslr: Randomize physical and virtual address of kernel separately Baoquan He
2016-03-07 23:51 ` Kees Cook
2016-03-08 5:34 ` Baoquan He
2016-03-08 18:24 ` Kees Cook
2016-03-09 13:40 ` Baoquan He
2016-03-09 18:07 ` Kees Cook
2016-03-10 15:15 ` Baoquan He
2016-03-04 16:25 ` [PATCH v3 17/19] x86, kaslr: Add support of kernel physical address randomization above 4G Baoquan He
2016-03-04 16:25 ` [PATCH v3 18/19] x86, kaslr: Remove useless codes Baoquan He
2016-03-04 16:25 ` [PATCH v3 19/19] x86, kaslr: Allow random address to be below loaded address Baoquan He
2016-03-05 11:35 ` [PATCH v3 00/19] x86, boot: kaslr cleanup and 64bit kaslr support Baoquan He
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20160308045010.GA2481@x1.redhat.com \
--to=bhe@redhat.com \
--cc=akpm@linux-foundation.org \
--cc=bp@alien8.de \
--cc=dyoung@redhat.com \
--cc=hpa@zytor.com \
--cc=keescook@chromium.org \
--cc=lasse.collin@tukaani.org \
--cc=linux-kernel@vger.kernel.org \
--cc=luto@kernel.org \
--cc=mingo@redhat.com \
--cc=vgoyal@redhat.com \
--cc=yinghai@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).