linux-kernel - Re: [PATCH 08/17] Generalize relocate_kernel() for use by other architectures.

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 18 Sep 2013 09:31:21 -0700
From:	Roy Franz <roy.franz@...aro.org>
To:	Matt Fleming <matt@...sole-pimps.org>
Cc:	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	linux-efi@...r.kernel.org, matt.fleming@...el.com,
	Leif Lindholm <leif.lindholm@...aro.org>,
	Mark Salter <msalter@...hat.com>
Subject: Re: [PATCH 08/17] Generalize relocate_kernel() for use by other architectures.

On Wed, Sep 18, 2013 at 5:12 AM, Matt Fleming <matt@...sole-pimps.org> wrote:
> On Mon, 16 Sep, at 09:11:24PM, Roy Franz wrote:
>> Rename relocate_kernel() to efi_relocate_kernel(), and take
>> parameters rather than x86 specific structure.  Add max_addr
>> argument as for ARM we have some address constraints that we
>> need to enforce when relocating the kernel.  Add alloc_size
>> parameter for use by ARM64 which uses an uncompressed kernel,
>> and needs to allocate space for BSS.
>>
>> Signed-off-by: Roy Franz <roy.franz@...aro.org>
>> ---
>>  arch/x86/boot/compressed/eboot.c       |   12 ++++--
>>  drivers/firmware/efi/efi-stub-helper.c |   74 ++++++++++++++++++++++----------
>>  2 files changed, 60 insertions(+), 26 deletions(-)
>
> This commit breaks my ASUS machine.
>
>> -     /*
>> -      * The EFI firmware loader could have placed the kernel image
>> -      * anywhere in memory, but the kernel has various restrictions
>> -      * on the max physical address it can run at. Attempt to move
>> -      * the kernel to boot_params.pref_address, or as low as
>> -      * possible.
>> -      */
>
> [...]
>
>> +
>> +     /* The EFI firmware loader could have placed the kernel image
>> +      * anywhere in memory, but the kernel has restrictions on the
>> +      * anywhere in memory, but the kernel has restrictions on the
>> +      * max physical address it can run at.  Some architectures
>> +      * also have a prefered address, so first try to relocate
>> +      * to the preferred address.
>> +      */
>
> Please don't change the comment style of code in the future.
Sorry about that.
>
>> +     nr_pages = round_up(alloc_size, EFI_PAGE_SIZE) / EFI_PAGE_SIZE;
>> +     status = efi_call_phys4(sys_table_arg->boottime->allocate_pages,
>>                               EFI_ALLOCATE_ADDRESS, EFI_LOADER_DATA,
>> -                             nr_pages, &start);
>> +                             nr_pages, &efi_addr);
>> +     new_addr = efi_addr;
>> +     /* If preferred address allocation failed allocate as low as
>> +      * possible. */
>>       if (status != EFI_SUCCESS) {
>> -             status = efi_low_alloc(sys_table, hdr->init_size,
>> -                                hdr->kernel_alignment, &start);
>> -             if (status != EFI_SUCCESS)
>> -                     efi_printk(sys_table, "Failed to alloc mem for kernel\n");
>> +             status = efi_low_alloc(sys_table_arg, alloc_size, 0,
>> +                                    &new_addr);
>
> This is wrong. You've dropped the hdr->kernel_alignment argument, and
> the relaxed alignment of this allocation stops my machine from booting.
Dropping the alignment was an oversight, and my testing on OVMF didn't
catch that.  I'll add the alignment
and resend the series.

Thanks,
Roy

>
> --
> Matt Fleming, Intel Open Source Technology Center
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/