Re: [PATCH v7 8/9] cmd: efidebug: add uri device path

[Ilias Apalodimas <ilias.apalodimas@linaro.org>]

Hi Kojima-san,

[...]

> +			id = (int)hextoul(argv[1], &endp);
> +			if (*endp != '\0' || id > 0xffff)
> +				return CMD_RET_USAGE;
> +
> +			efi_create_indexed_name(var_name16, sizeof(var_name16),
> +						"Boot", id);
> +
> +			label = efi_convert_string(argv[2]);
> +			if (!label)
> +				return CMD_RET_FAILURE;
> +			lo.label = label;
> +
> +			uridp_len = sizeof(struct efi_device_path) + strlen(argv[3]) + 1;
> +			fp_free = efi_alloc(uridp_len + sizeof(END));
> +			uridp = (struct efi_device_path_uri *)fp_free;
> +			uridp->dp.type = DEVICE_PATH_TYPE_MESSAGING_DEVICE;
> +			uridp->dp.sub_type = DEVICE_PATH_SUB_TYPE_MSG_URI;
> +			uridp->dp.length = uridp_len;
> +			if (!wget_validate_uri(argv[3])) {
> +				printf("ERROR: invalid URI\n");
> +				r = CMD_RET_FAILURE;
> +				goto out;
> +			}

This needs to be moved right under label = efi_convert_string(argv[2]);
and group all the checks together early.  You won't have to allocate and
free the memory as well 

> +
> +			strcpy(uridp->uri, argv[3]);
> +			pos = (char *)uridp + uridp_len;
> +			memcpy(pos, &END, sizeof(END));
> +			fp_size += uridp_len + sizeof(END);

Assign this earlier and use it on fp_free = efi_alloc(...)

> +			file_path = (struct efi_device_path *)uridp;
> +			argc -= 3;

[...]

> + *
> + * @uri:	uri string of target file of wget
> + * Return:	true if uri is valid, false if uri is invalid
> + */
> +bool wget_validate_uri(char *uri);
> +
>  #endif /* __NET_H__ */
> diff --git a/net/wget.c b/net/wget.c
> index 2087146b37..6ae2237a0a 100644
> --- a/net/wget.c
> +++ b/net/wget.c
> @@ -566,3 +566,74 @@ out:
>  	return ret;
>  }
>  #endif
> +
 
Regards
/Ilias