linux-kernel - Re: [PATCH v16 1/6] efi: ARM/arm64: ignore DT memory nodes instead of removing them

Open Source and information security mailing list archives

Message-ID: <CAKv+Gu_aed0N9hiopY_=9P_y1rZ8ou6ofuHKdkXJnY=v1Gzh_w@mail.gmail.com>
Date:	Thu, 14 Apr 2016 13:10:35 +0200
From:	Ard Biesheuvel <ard.biesheuvel@...aro.org>
To:	Steve Capper <steve.capper@....com>
Cc:	David Daney <ddaney.cavm@...il.com>,
	Will Deacon <will.deacon@....com>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>,
	Rob Herring <robh+dt@...nel.org>,
	Frank Rowand <frowand.list@...il.com>,
	Grant Likely <grant.likely@...aro.org>,
	Pawel Moll <pawel.moll@....com>,
	Ian Campbell <ijc+devicetree@...lion.org.uk>,
	Kumar Gala <galak@...eaurora.org>,
	Ganapatrao Kulkarni <gkulkarni@...iumnetworks.com>,
	Robert Richter <rrichter@...ium.com>,
	Matt Fleming <matt@...eblueprint.co.uk>,
	Mark Rutland <mark.rutland@....com>,
	Catalin Marinas <catalin.marinas@....com>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
	"linux-efi@...r.kernel.org" <linux-efi@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	David Daney <david.daney@...ium.com>
Subject: Re: [PATCH v16 1/6] efi: ARM/arm64: ignore DT memory nodes instead of
 removing them

On 14 April 2016 at 13:02, Steve Capper <steve.capper@....com> wrote:
> On Fri, Apr 08, 2016 at 03:50:23PM -0700, David Daney wrote:
>> From: Ard Biesheuvel <ard.biesheuvel@...aro.org>
>>
>> There are two problems with the UEFI stub DT memory node removal
>> routine:
>> - it deletes nodes as it traverses the tree, which happens to work
>>   but is not supported, as deletion invalidates the node iterator;
>> - deleting memory nodes entirely may discard annotations in the form
>>   of additional properties on the nodes.
>>
>> Since the discovery of DT memory nodes occurs strictly before the
>> UEFI init sequence, we can simply clear the memblock memory table
>> before parsing the UEFI memory map. This way, it is no longer
>> necessary to remove the nodes, so we can remove that logic from the
>> stub as well.
>>
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@...aro.org>
>> Signed-off-by: David Daney <david.daney@...ium.com>
>> ---
>>  drivers/firmware/efi/arm-init.c    |  8 ++++++++
>>  drivers/firmware/efi/libstub/fdt.c | 24 +-----------------------
>>  2 files changed, 9 insertions(+), 23 deletions(-)
>>
>> diff --git a/drivers/firmware/efi/arm-init.c b/drivers/firmware/efi/arm-init.c
>> index aa1f743..5d6945b 100644
>> --- a/drivers/firmware/efi/arm-init.c
>> +++ b/drivers/firmware/efi/arm-init.c
>> @@ -143,6 +143,14 @@ static __init void reserve_regions(void)
>>       if (efi_enabled(EFI_DBG))
>>               pr_info("Processing EFI memory map:\n");
>>
>> +     /*
>> +      * Discard memblocks discovered so far: if there are any at this
>> +      * point, they originate from memory nodes in the DT, and UEFI
>> +      * uses its own memory map instead.
>> +      */
>> +     memblock_dump_all();
>> +     memblock_remove(0, ULLONG_MAX);
>> +
>
> Does this change need to be applied to any other architectures given
> that deletion code has been removed from libstub below?
>

The 'generic' libstub code below is only used by ARM, so we're safe
here in that regard.

>>       for_each_efi_memory_desc(&memmap, md) {
>>               paddr = md->phys_addr;
>>               npages = md->num_pages;
>> diff --git a/drivers/firmware/efi/libstub/fdt.c b/drivers/firmware/efi/libstub/fdt.c
>> index 6dba78a..e58abfa 100644
>> --- a/drivers/firmware/efi/libstub/fdt.c
>> +++ b/drivers/firmware/efi/libstub/fdt.c
>> @@ -24,7 +24,7 @@ efi_status_t update_fdt(efi_system_table_t *sys_table, void *orig_fdt,
>>                       unsigned long map_size, unsigned long desc_size,
>>                       u32 desc_ver)
>>  {
>> -     int node, prev, num_rsv;
>> +     int node, num_rsv;
>>       int status;
>>       u32 fdt_val32;
>>       u64 fdt_val64;
>> @@ -54,28 +54,6 @@ efi_status_t update_fdt(efi_system_table_t *sys_table, void *orig_fdt,
>>               goto fdt_set_fail;
>>
>>       /*
>> -      * Delete any memory nodes present. We must delete nodes which
>> -      * early_init_dt_scan_memory may try to use.
>> -      */
>> -     prev = 0;
>> -     for (;;) {
>> -             const char *type;
>> -             int len;
>> -
>> -             node = fdt_next_node(fdt, prev, NULL);
>> -             if (node < 0)
>> -                     break;
>> -
>> -             type = fdt_getprop(fdt, node, "device_type", &len);
>> -             if (type && strncmp(type, "memory", len) == 0) {
>> -                     fdt_del_node(fdt, node);
>> -                     continue;
>> -             }
>> -
>> -             prev = node;
>> -     }
>> -
>> -     /*
>>        * Delete all memory reserve map entries. When booting via UEFI,
>>        * kernel will use the UEFI memory map to find reserved regions.
>>        */
>> --
>> 1.8.3.1
>>
>>
>> _______________________________________________
>> linux-arm-kernel mailing list
>> linux-arm-kernel@...ts.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>>

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