Re: [PATCH 1/2] cmd: efidebug: add uri device path

[Heinrich Schuchardt <xypron.glpk@gmx.de>]

On 8/23/23 10:37, Masahisa Kojima wrote:
> This adds the URI device path option for 'boot add' subcommand.
> User can add the URI load option for downloading ISO image file
> or EFI application through network(e.g. HTTP).
>
> Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>
> ---
>   cmd/efidebug.c | 39 +++++++++++++++++++++++++++++++++++++++
>   1 file changed, 39 insertions(+)
>
> diff --git a/cmd/efidebug.c b/cmd/efidebug.c
> index 0be3af3e76..62f867df2a 100644
> --- a/cmd/efidebug.c
> +++ b/cmd/efidebug.c
> @@ -829,6 +829,44 @@ static int do_efi_boot_add(struct cmd_tbl *cmdtp, int flag,
>   			argc -= 1;
>   			argv += 1;
>   			break;
> +		case 'u':
> +		{
> +			char *pos;
> +			int uridp_len;
> +			struct efi_device_path_uri *uridp;
> +
> +			if (argc <  3 || lo.label) {
> +				r = CMD_RET_USAGE;
> +				goto out;
> +			}
> +			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;
> +			strcpy(uridp->uri, argv[3]);

This assumes that argv[3] is a valid URI.

Would it be preferable to validate that the string is percent encoded,
conforms to RFC 3986, and contains a supported scheme, an authority, and
a path?

As a user I would like related errors to be caught at entry and not at
runtime.

Best regards

Heinrich

> +			pos = (char *)uridp + uridp_len;
> +			memcpy(pos, &END, sizeof(END));
> +			fp_size += uridp_len + sizeof(END);
> +			file_path = (struct efi_device_path *)uridp;
> +			argc -= 3;
> +			argv += 3;
> +			break;
> +		}
> +
>   		default:
>   			r = CMD_RET_USAGE;
>   			goto out;
> @@ -1492,6 +1530,7 @@ static char efidebug_help_text[] =
>   	"  -b|-B <bootid> <label> <interface> <devnum>[:<part>] <file path>\n"
>   	"  -i|-I <interface> <devnum>[:<part>] <initrd file path>\n"
>   	"  (-b, -i for short form device path)\n"
> +	"  -u <bootid> <label> <uri>\n"
>   	"  -s '<optional data>'\n"
>   	"efidebug boot rm <bootid#1> [<bootid#2> [<bootid#3> [...]]]\n"
>   	"  - delete UEFI BootXXXX variables\n"