| 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: <CAGXu5jLd7wVd7t8HNmYAKaCGgYJp8C_aHNs6F3n3yj=4AF82yQ@mail.gmail.com>
Date: Mon, 7 Mar 2016 15:28:26 -0800
From: Kees Cook <keescook@...omium.org>
To: Baoquan He <bhe@...hat.com>
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 08/19] x86, kaslr: Consolidate mem_avoid array filling
On Fri, Mar 4, 2016 at 8:25 AM, Baoquan He <bhe@...hat.com> wrote:
> From: Yinghai Lu <yinghai@...nel.org>
>
> We are going to support kaslr with 64bit above 4G, and new random output
> could be anywhere. Array mem_avoid is used for kaslr to search new output
> address. Current code only track range that is after output+output_size.
> So we need to track all ranges instead of just after output+output_size.
>
> Current code has first entry which is extra bytes before input+input_size,
> and it is according to output_size. Other entries are for initrd, cmdline,
> and heap/stack for ZO running.
>
> At first, let's check the first entry that should be in the mem_avoid array.
> Now ZO sit end of the buffer always, we can find out where is text and
> data/bss etc of ZO.
>
> Since init_size >= run_size, and input+input_len >= output+output_len,
> here make several assumptions for better presentation by graph:
> - init_size > run_size
> - input+input_len > output+output_len
> - run_size > output_len
I would like to see each of these assumptions justified. Why is
init_size > run_size, etc?
choose_kernel_location's "output_size" is calculated as max(run_size,
output_len), so run_size may not be > output_len...
>
> 0 output input input+input_len output+init_size
> | | | | |
> |-----|-------------------|--------|------------------|----|------------|----------|
> | | |
> output+init_size-ZO_INIT_SIZE output+output_len output+run_size
>
> [output, output+init_size) is the for decompressing buffer for compressed
> kernel.
>
> [output, output+run_size) is for VO run size.
> [output, output+output_len) is (VO (vmlinux after objcopy) plus relocs)
>
> [output+init_size-ZO_INIT_SIZE, output+init_size) is copied ZO.
> [input, input+input_len) is copied compressed (VO (vmlinux after objcopy)
> plus relocs), not the ZO.
>
> [input+input_len, output+init_size) is [_text, _end) for ZO. that could be
> first range in mem_avoid. Now the new first entry already includes heap and
> stack for ZO running. So no need to put them separately into mem_avoid array.
>
> Also [input, input+input_size) need be put in mem_avoid array. It is adjacent
> to new first entry, so merge them.
I wonder if this diagram and description should live in a comment with the code.
-Kees
>
> At last we need to put boot_params into the mem_avoid too. As with 64bit
> bootloader could put it anywhere.
>
> After these changes, we have all ranges which need be avoided in mem_avoid
> array.
>
> Cc: Kees Cook <keescook@...omium.org>
> Signed-off-by: Yinghai Lu <yinghai@...nel.org>
> ---
> v2->v3:
> Adjust the patch log.
>
> arch/x86/boot/compressed/aslr.c | 29 +++++++++++++----------------
> 1 file changed, 13 insertions(+), 16 deletions(-)
>
> diff --git a/arch/x86/boot/compressed/aslr.c b/arch/x86/boot/compressed/aslr.c
> index 622aa88..b93be03 100644
> --- a/arch/x86/boot/compressed/aslr.c
> +++ b/arch/x86/boot/compressed/aslr.c
> @@ -109,7 +109,7 @@ struct mem_vector {
> unsigned long size;
> };
>
> -#define MEM_AVOID_MAX 5
> +#define MEM_AVOID_MAX 4
> static struct mem_vector mem_avoid[MEM_AVOID_MAX];
>
> static bool mem_contains(struct mem_vector *region, struct mem_vector *item)
> @@ -135,21 +135,22 @@ static bool mem_overlaps(struct mem_vector *one, struct mem_vector *two)
> }
>
> static void mem_avoid_init(unsigned long input, unsigned long input_size,
> - unsigned long output, unsigned long output_size)
> + unsigned long output)
> {
> + unsigned long init_size = real_mode->hdr.init_size;
> u64 initrd_start, initrd_size;
> u64 cmd_line, cmd_line_size;
> - unsigned long unsafe, unsafe_len;
> char *ptr;
>
> /*
> * Avoid the region that is unsafe to overlap during
> - * decompression (see calculations at top of misc.c).
> + * decompression.
> + * As we already move ZO (arch/x86/boot/compressed/vmlinux)
> + * to the end of buffer, [input+input_size, output+init_size)
> + * has [_text, _end) for ZO.
> */
> - unsafe_len = (output_size >> 12) + 32768 + 18;
> - unsafe = (unsigned long)input + input_size - unsafe_len;
> - mem_avoid[0].start = unsafe;
> - mem_avoid[0].size = unsafe_len;
> + mem_avoid[0].start = input;
> + mem_avoid[0].size = (output + init_size) - input;
>
> /* Avoid initrd. */
> initrd_start = (u64)real_mode->ext_ramdisk_image << 32;
> @@ -169,13 +170,9 @@ static void mem_avoid_init(unsigned long input, unsigned long input_size,
> mem_avoid[2].start = cmd_line;
> mem_avoid[2].size = cmd_line_size;
>
> - /* Avoid heap memory. */
> - mem_avoid[3].start = (unsigned long)free_mem_ptr;
> - mem_avoid[3].size = BOOT_HEAP_SIZE;
> -
> - /* Avoid stack memory. */
> - mem_avoid[4].start = (unsigned long)free_mem_end_ptr;
> - mem_avoid[4].size = BOOT_STACK_SIZE;
> + /* Avoid params */
> + mem_avoid[3].start = (unsigned long)real_mode;
> + mem_avoid[3].size = sizeof(*real_mode);
> }
>
> /* Does this memory vector overlap a known avoided area? */
> @@ -319,7 +316,7 @@ unsigned char *choose_kernel_location(unsigned char *input,
>
> /* Record the various known unsafe memory ranges. */
> mem_avoid_init((unsigned long)input, input_size,
> - (unsigned long)output, output_size);
> + (unsigned long)output);
>
> /* Walk e820 and find a random address. */
> random = find_random_addr(choice, output_size);
> --
> 2.5.0
>
--
Kees Cook
Chrome OS & Brillo Security
Powered by blists - more mailing lists