Re: [PATCH v4 1/4] x86/boot/KASLR: Wrap e820 entries walking code into new function process_e820_entries()

[Kees Cook]

On Sun, Jul 9, 2017 at 5:37 AM, Baoquan He <bhe@xxxxxxxxxx> wrote:
> The original function process_e820_entry() only takes care of each
> e820 entry passed.
>
> And move the E820_TYPE_RAM checking logic into process_e820_entries().
>
> And remove the redundent local variable 'addr' definition in
> find_random_phys_addr().
>
> Signed-off-by: Baoquan He <bhe@xxxxxxxxxx>

Acked-by: Kees Cook <keescook@xxxxxxxxxxxx>

-Kees

> ---
>  arch/x86/boot/compressed/kaslr.c | 38 +++++++++++++++++++++-----------------
>  1 file changed, 21 insertions(+), 17 deletions(-)
>
> diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c
> index 91f27ab970ef..1485f48aeda1 100644
> --- a/arch/x86/boot/compressed/kaslr.c
> +++ b/arch/x86/boot/compressed/kaslr.c
> @@ -488,10 +488,6 @@ static void process_e820_entry(struct boot_e820_entry *entry,
>         unsigned long start_orig, end;
>         struct boot_e820_entry cur_entry;
>
> -       /* Skip non-RAM entries. */
> -       if (entry->type != E820_TYPE_RAM)
> -               return;
> -
>         /* On 32-bit, ignore entries entirely above our maximum. */
>         if (IS_ENABLED(CONFIG_X86_32) && entry->addr >= KERNEL_IMAGE_SIZE)
>                 return;
> @@ -562,12 +558,29 @@ static void process_e820_entry(struct boot_e820_entry *entry,
>         }
>  }
>
> -static unsigned long find_random_phys_addr(unsigned long minimum,
> -                                          unsigned long image_size)
> +static void process_e820_entries(unsigned long minimum,
> +                                unsigned long image_size)
>  {
>         int i;
> -       unsigned long addr;
> +       struct boot_e820_entry *entry;
> +
> +       /* Verify potential e820 positions, appending to slots list. */
> +       for (i = 0; i < boot_params->e820_entries; i++) {
> +               entry = &boot_params->e820_table[i];
> +               /* Skip non-RAM entries. */
> +               if (entry->type != E820_TYPE_RAM)
> +                       continue;
> +               process_e820_entry(entry, minimum, image_size);
> +               if (slot_area_index == MAX_SLOT_AREA) {
> +                       debug_putstr("Aborted e820 scan (slot_areas full)!\n");
> +                       break;
> +               }
> +       }
> +}
>
> +static unsigned long find_random_phys_addr(unsigned long minimum,
> +                                          unsigned long image_size)
> +{
>         /* Check if we had too many memmaps. */
>         if (memmap_too_large) {
>                 debug_putstr("Aborted e820 scan (more than 4 memmap= args)!\n");
> @@ -577,16 +590,7 @@ static unsigned long find_random_phys_addr(unsigned long minimum,
>         /* Make sure minimum is aligned. */
>         minimum = ALIGN(minimum, CONFIG_PHYSICAL_ALIGN);
>
> -       /* Verify potential e820 positions, appending to slots list. */
> -       for (i = 0; i < boot_params->e820_entries; i++) {
> -               process_e820_entry(&boot_params->e820_table[i], minimum,
> -                                  image_size);
> -               if (slot_area_index == MAX_SLOT_AREA) {
> -                       debug_putstr("Aborted e820 scan (slot_areas full)!\n");
> -                       break;
> -               }
> -       }
> -
> +       process_e820_entries(minimum, image_size);
>         return slots_fetch_random();
>  }
>
> --
> 2.5.5
>



-- 
Kees Cook
Pixel Security