Re: [PATCH 3/3] efi/libstub: fix deletion of FDT memory nodes

[Mark Salter <msalter-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>]

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-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---
>  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.


>  			fdt_del_node(fdt, node);
> -			continue;
> +			goto restart;
>  		}
>  
>  		prev = node;

Maybe do:

 		if (type && strncmp(type, "memory", len) == 0) {
 			fdt_del_node(fdt, node);
-			continue;
-		}
-
-		prev = node;
+			prev = NULL;
+		} else
+			prev = node;
 	}


instead of adding a goto.