Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1350427 > unrolled thread

[PATCH v3 08/19] x86, kaslr: Consolidate mem_avoid array filling

Started byBaoquan He <bhe@redhat.com>
First post2016-03-04 17:30 +0100
Last post2016-03-09 14:50 +0100
Articles 5 — 2 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH v3 08/19] x86, kaslr: Consolidate mem_avoid array filling Baoquan He <bhe@redhat.com> - 2016-03-04 17:30 +0100
    Re: [PATCH v3 08/19] x86, kaslr: Consolidate mem_avoid array filling Kees Cook <keescook@chromium.org> - 2016-03-08 00:30 +0100
      Re: [PATCH v3 08/19] x86, kaslr: Consolidate mem_avoid array filling Baoquan He <bhe@redhat.com> - 2016-03-08 06:30 +0100
        Re: [PATCH v3 08/19] x86, kaslr: Consolidate mem_avoid array filling Kees Cook <keescook@chromium.org> - 2016-03-08 19:20 +0100
          Re: [PATCH v3 08/19] x86, kaslr: Consolidate mem_avoid array filling Baoquan He <bhe@redhat.com> - 2016-03-09 14:50 +0100

#1350427 — [PATCH v3 08/19] x86, kaslr: Consolidate mem_avoid array filling

FromBaoquan He <bhe@redhat.com>
Date2016-03-04 17:30 +0100
Subject[PATCH v3 08/19] x86, kaslr: Consolidate mem_avoid array filling
Message-ID<r90M3-Y1-53@gated-at.bofh.it>
From: Yinghai Lu <yinghai@kernel.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

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.

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@chromium.org>
Signed-off-by: Yinghai Lu <yinghai@kernel.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

[toc] | [next] | [standalone]

#1352171

FromKees Cook <keescook@chromium.org>
Date2016-03-08 00:30 +0100
Message-ID<racL9-13y-27@gated-at.bofh.it>
In reply to#1350427
On Fri, Mar 4, 2016 at 8:25 AM, Baoquan He <bhe@redhat.com> wrote:
> From: Yinghai Lu <yinghai@kernel.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@chromium.org>
> Signed-off-by: Yinghai Lu <yinghai@kernel.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

[toc] | [prev] | [next] | [standalone]

#1352637

FromBaoquan He <bhe@redhat.com>
Date2016-03-08 06:30 +0100
Message-ID<rainv-4Ut-3@gated-at.bofh.it>
In reply to#1352171
On 03/07/16 at 03:28pm, Kees Cook wrote:
> On Fri, Mar 4, 2016 at 8:25 AM, Baoquan He <bhe@redhat.com> wrote:
> > From: Yinghai Lu <yinghai@kernel.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...

Sure. I will add this case in next post. Thanks a lot.
> 
> >
> > 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.

I think it would be very helpful for people interested in this process.
Do you think it's ok to put it where init_size is calculated in
boot/header.S?  Or other suitable places?
> 
> 
> >
> > 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@chromium.org>
> > Signed-off-by: Yinghai Lu <yinghai@kernel.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

[toc] | [prev] | [next] | [standalone]

#1353287

FromKees Cook <keescook@chromium.org>
Date2016-03-08 19:20 +0100
Message-ID<rauoG-4zc-5@gated-at.bofh.it>
In reply to#1352637
On Mon, Mar 7, 2016 at 9:21 PM, Baoquan He <bhe@redhat.com> wrote:
> On 03/07/16 at 03:28pm, Kees Cook wrote:
>> On Fri, Mar 4, 2016 at 8:25 AM, Baoquan He <bhe@redhat.com> wrote:
>> > From: Yinghai Lu <yinghai@kernel.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...
>
> Sure. I will add this case in next post. Thanks a lot.
>>
>> >
>> > 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.
>
> I think it would be very helpful for people interested in this process.
> Do you think it's ok to put it where init_size is calculated in
> boot/header.S?  Or other suitable places?

Let's put it in aslr.c since it's the biggest user of these
calculations? I would tend to avoid putting large comments in a .S
file, but that's just personal preference.

-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@chromium.org>
>> > Signed-off-by: Yinghai Lu <yinghai@kernel.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



-- 
Kees Cook
Chrome OS & Brillo Security

[toc] | [prev] | [next] | [standalone]

#1354131

FromBaoquan He <bhe@redhat.com>
Date2016-03-09 14:50 +0100
Message-ID<raMEW-9f-13@gated-at.bofh.it>
In reply to#1353287
On 03/08/16 at 10:17am, Kees Cook wrote:
> On Mon, Mar 7, 2016 at 9:21 PM, Baoquan He <bhe@redhat.com> wrote:
> > On 03/07/16 at 03:28pm, Kees Cook wrote:
> >> On Fri, Mar 4, 2016 at 8:25 AM, Baoquan He <bhe@redhat.com> wrote:
> >> > From: Yinghai Lu <yinghai@kernel.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...
> >
> > Sure. I will add this case in next post. Thanks a lot.
> >>
> >> >
> >> > 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.
> >
> > I think it would be very helpful for people interested in this process.
> > Do you think it's ok to put it where init_size is calculated in
> > boot/header.S?  Or other suitable places?
> 
> Let's put it in aslr.c since it's the biggest user of these
> calculations? I would tend to avoid putting large comments in a .S
> file, but that's just personal preference.

Yeah, agree. Will put it in aslr.c.

Thanks a lot for your good suggestions.

>

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web