On 7 October 2015 at 18:56, Ard Biesheuvel <ard.biesheuvel@xxxxxxxxxx> wrote: > On 7 October 2015 at 17:02, Ard Biesheuvel <ard.biesheuvel@xxxxxxxxxx> wrote: >> On 7 October 2015 at 16:49, Mark Salter <msalter@xxxxxxxxxx> wrote: >>> On Wed, 2015-10-07 at 09:35 +0100, Ard Biesheuvel wrote: >>>> This fixes two issues with the EFI libstub code that removes the memory >>>> nodes from the FDT: >>>> a) fdt_del_node() invalidates the iterator, so we should restart from the >>>> top after calling it; >>>> b) use fixed length of 6 when matching against 'memory' using strncmp(), >>>> since otherwise, substrings like 'm' or 'mem' could match as well. >>>> >>>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@xxxxxxxxxx> >>>> --- >>>> drivers/firmware/efi/libstub/fdt.c | 5 +++-- >>>> 1 file changed, 3 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/drivers/firmware/efi/libstub/fdt.c b/drivers/firmware/efi/libstub/fdt.c >>>> index 0ef16923b4f0..2c7d09936479 100644 >>>> --- a/drivers/firmware/efi/libstub/fdt.c >>>> +++ b/drivers/firmware/efi/libstub/fdt.c >>>> @@ -200,6 +200,7 @@ efi_status_t update_fdt(efi_system_table_t *sys_table, void *orig_fdt, >>>> * Delete any memory nodes present. We must delete nodes which >>>> * early_init_dt_scan_memory may try to use. >>>> */ >>>> +restart: >>>> prev = 0; >>>> for (;;) { >>>> const char *type; >>>> @@ -210,9 +211,9 @@ efi_status_t update_fdt(efi_system_table_t *sys_table, void *orig_fdt, >>>> break; >>>> >>>> type = fdt_getprop(fdt, node, "device_type", &len); >>>> - if (type && strncmp(type, "memory", len) == 0) { >>>> + if (type && strncmp(type, "memory", 6) == 0) { >>> >>> I see what you're trying to fix here, but I think using 6 is wrong >>> also. A plain strcmp() would work as long as property is a string. >>> >> >> That is what Leif suggested as well, but it pulls in yet another >> string function into the stub, so I tried to avoid it. I think this >> particular case is safe, since the property names are in the string >> table, which is after the nodes in the DTB structure, so 'type' is >> guaranteed not to point right before the end of a mapped region. >> > > ... or is your concern about matching on longer strings that only > start with 'memory'? > > I guess it makes sense to use strcmp() after all, only arm64 does not > have an asm implementation, so under the stricter rules that I am > proposing for fencing off the EFI stub code from the kernel proper, I > will have to go and add a strcmp() implementation to > drivers/firmware/efi/libstub/string.c. > OK, strike that. It does have an asm implementation, I just have to expose it to the stub, Thanks, Ard. -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html