[PATCH] cmd: eficonfig: add support for URI device path based boot options

[Heinrich Schuchardt]

On 6/16/25 12:23, Sughosh Ganu wrote:
> The eficonfig command provides a menu based interface for maintenance
> of the EFI boot options. Add support for adding a URI based boot
> option. This boot option can then be used for HTTP boot.

With your patch when I try to add a boot option I was surprised to see 
this menu:

   ** Add Boot Option **

       Description:
       File:
       Initrd File:
       Fdt File:
       Optional Data:
       Uri:
       Save
       Quit

My expectation is that I can define a URL each for

* binary
* initrd
* fdt

It should be possible to choose a network device in this menu instead:

   ** Select Volume **

       nvme 0:1
       nvme 0:2
       mmc 0:1
       mmc 0:2
       Quit

> 
> Signed-off-by: Sughosh Ganu <sughosh.ganu at linaro.org>
> ---
> 
> Note: I had explored adding a corresponding test for this change. But
> looks like the eficonfig command itself is not being tested since
> commit 952018117ab4 ("dm: sandbox: Switch over to using the new host
> uclass"). This needs to be taken up separately as to why the eficonfig
> test has been bypassed, before adding stuff to the test.
> 
> 
>   cmd/eficonfig.c | 90 +++++++++++++++++++++++++++++++++++++++++++++++--
>   1 file changed, 87 insertions(+), 3 deletions(-)
> 
> diff --git a/cmd/eficonfig.c b/cmd/eficonfig.c
> index 6e14d34a6bd..f69902b76f7 100644
> --- a/cmd/eficonfig.c
> +++ b/cmd/eficonfig.c
> @@ -35,6 +35,7 @@ static int avail_row;
>   
>   #define EFICONFIG_DESCRIPTION_MAX 32
>   #define EFICONFIG_OPTIONAL_DATA_MAX 64
> +#define EFICONFIG_URI_MAX 512
>   #define EFICONFIG_MENU_HEADER_ROW_NUM 3
>   #define EFICONFIG_MENU_DESC_ROW_NUM 5
>   
> @@ -57,6 +58,7 @@ struct eficonfig_filepath_info {
>    * @boot_index:		index of the boot option
>    * @description:	pointer to the description string
>    * @optional_data:	pointer to the optional_data
> + * @uri:		URI for HTTP Boot
>    * @edit_completed:	flag indicates edit complete
>    */
>   struct eficonfig_boot_option {
> @@ -66,6 +68,7 @@ struct eficonfig_boot_option {
>   	unsigned int boot_index;
>   	u16 *description;
>   	u16 *optional_data;
> +	u16 *uri;
>   	bool edit_completed;
>   };
>   
> @@ -538,6 +541,31 @@ struct efi_device_path *eficonfig_create_device_path(struct efi_device_path *dp_
>   	return dp;
>   }
>   
> +static struct efi_device_path *eficonfig_create_uri_device_path(u16 *uri_str)
> +{
> +	char *pos, *p;
> +	u32 len = 0;
> +	efi_uintn_t uridp_len;
> +	struct efi_device_path_uri *uridp;
> +
> +	len = utf16_utf8_strlen(uri_str);

The code in this function seems not to guarantee valid URLs. E.g. you 
don't check that the schema is http:// or https://.

Please, provide unit tests for the URL validation.

The UEFI spec refers to RFC 3986 for the allowable content of a Uri() 
device-path.

See also https://url.spec.whatwg.org/#valid-url-string

The Chinese URL https://www.hk01.com/zone/1/港聞 should be encoded as 
https://www.hk01.com/zone/1/%E6%B8%AF%E8%81%9E.

The following steps are required:

1) convert to UTF-8
2) encode bytes in URL fragements using hex encoding as needed.

See https://url.spec.whatwg.org/#url-fragment-string

Please, implement unit tests for URL encoding.

Best regards

Heinrich

> +
> +	uridp_len = sizeof(struct efi_device_path) + len + 1;
> +	uridp = efi_alloc(uridp_len + sizeof(EFI_DP_END));
> +	if (!uridp)
> +		return NULL;
> +
> +	uridp->dp.type = DEVICE_PATH_TYPE_MESSAGING_DEVICE;
> +	uridp->dp.sub_type = DEVICE_PATH_SUB_TYPE_MSG_URI;
> +	uridp->dp.length = uridp_len;
> +	p = (char *)&uridp->uri;
> +	utf16_utf8_strcpy(&p, uri_str);
> +	pos = (char *)uridp + uridp_len;
> +	memcpy(pos, &EFI_DP_END, sizeof(EFI_DP_END));
> +
> +	return &uridp->dp;
> +}
> +
>   /**
>    * eficonfig_file_selected() - handler of file selection
>    *
> @@ -983,6 +1011,22 @@ static efi_status_t eficonfig_boot_add_optional_data(void *data)
>   				 "  enter optional data:");
>   }
>   
> +/**
> + * eficonfig_boot_add_uri() - handle user input for HTTP Boot URI
> + *
> + * @data:	pointer to the internal boot option structure
> + * Return:	status code
> + */
> +static efi_status_t eficonfig_boot_add_uri(void *data)
> +{
> +	struct eficonfig_boot_option *bo = data;
> +
> +	return handle_user_input(bo->uri, EFICONFIG_URI_MAX, 24,
> +				 "\n  ** Edit URI **\n"
> +				 "\n"
> +				 "  enter HTTP Boot URI:");
> +}
> +
>   /**
>    * eficonfig_boot_edit_save() - handler to save the boot option
>    *
> @@ -998,7 +1042,8 @@ static efi_status_t eficonfig_boot_edit_save(void *data)
>   		bo->edit_completed = false;
>   		return EFI_NOT_READY;
>   	}
> -	if (u16_strlen(bo->file_info.current_path) == 0) {
> +	if (u16_strlen(bo->file_info.current_path) == 0 &&
> +	    u16_strlen(bo->uri) == 0) {
>   		eficonfig_print_msg("File is not selected!");
>   		bo->edit_completed = false;
>   		return EFI_NOT_READY;
> @@ -1318,6 +1363,11 @@ static efi_status_t eficonfig_show_boot_option(struct eficonfig_boot_option *bo,
>   	if (ret != EFI_SUCCESS)
>   		goto out;
>   
> +	ret = create_boot_option_entry(efi_menu, "Uri: ", bo->uri,
> +				       eficonfig_boot_add_uri, bo);
> +	if (ret != EFI_SUCCESS)
> +		goto out;
> +
>   	ret = create_boot_option_entry(efi_menu, "Save", NULL,
>   				       eficonfig_boot_edit_save, bo);
>   	if (ret != EFI_SUCCESS)
> @@ -1340,6 +1390,22 @@ out:
>   	return ret;
>   }
>   
> +static bool eficonfig_fp_uri(struct efi_device_path *dp)
> +{
> +	return dp->type == DEVICE_PATH_TYPE_MESSAGING_DEVICE &&
> +		dp->sub_type == DEVICE_PATH_SUB_TYPE_MSG_URI;
> +}
> +
> +static void fill_dp_uri(struct efi_device_path *dp, u16 **uri_str)
> +{
> +	u16 *p = *uri_str;
> +	struct efi_device_path_uri *uridp;
> +
> +	uridp = (struct efi_device_path_uri *)dp;
> +
> +	utf8_utf16_strcpy(&p, uridp->uri);
> +}
> +
>   /**
>    * fill_file_info() - fill the file info from efi_device_path structure
>    *
> @@ -1392,10 +1458,13 @@ static efi_status_t eficonfig_edit_boot_option(u16 *varname, struct eficonfig_bo
>   	size_t len;
>   	efi_status_t ret;
>   	char *tmp = NULL, *p;
> +	u16 *current_path = NULL;
>   	struct efi_load_option lo = {0};
>   	efi_uintn_t dp_size;
>   	struct efi_device_path *dp = NULL;
>   	efi_uintn_t size = load_option_size;
> +	struct efi_device_path *dp_volume = NULL;
> +	struct efi_device_path *uri_dp = NULL;
>   	struct efi_device_path *device_dp = NULL;
>   	struct efi_device_path *initrd_dp = NULL;
>   	struct efi_device_path *fdt_dp = NULL;
> @@ -1464,6 +1533,12 @@ static efi_status_t eficonfig_edit_boot_option(u16 *varname, struct eficonfig_bo
>   		goto out;
>   	}
>   
> +	bo->uri = calloc(1, EFICONFIG_URI_MAX * sizeof(u16));
> +	if (!bo->uri) {
> +		ret =  EFI_OUT_OF_RESOURCES;
> +		goto out;
> +	}
> +
>   	/* copy the preset value */
>   	if (load_option) {
>   		ret = efi_deserialize_load_option(&lo, load_option, &size);
> @@ -1481,7 +1556,9 @@ static efi_status_t eficonfig_edit_boot_option(u16 *varname, struct eficonfig_bo
>   		u16_strcpy(bo->description, lo.label);
>   
>   		/* EFI image file path is a first instance */
> -		if (lo.file_path)
> +		if (lo.file_path && eficonfig_fp_uri(lo.file_path))
> +			fill_dp_uri(lo.file_path, &bo->uri);
> +		else
>   			fill_file_info(lo.file_path, &bo->file_info, device_dp);
>   
>   		/* Initrd file path (optional) is placed at second instance. */
> @@ -1512,6 +1589,9 @@ static efi_status_t eficonfig_edit_boot_option(u16 *varname, struct eficonfig_bo
>   			goto out;
>   	}
>   
> +	if (utf16_utf8_strlen(bo->uri))
> +		uri_dp = eficonfig_create_uri_device_path(bo->uri);
> +
>   	if (bo->initrd_info.dp_volume) {
>   		dp = eficonfig_create_device_path(bo->initrd_info.dp_volume,
>   						 bo->initrd_info.current_path);
> @@ -1536,7 +1616,10 @@ static efi_status_t eficonfig_edit_boot_option(u16 *varname, struct eficonfig_bo
>   		efi_free_pool(dp);
>   	}
>   
> -	dp = eficonfig_create_device_path(bo->file_info.dp_volume, bo->file_info.current_path);
> +	dp_volume = bo->file_info.dp_volume;
> +	current_path = bo->file_info.current_path;
> +	dp = uri_dp ?
> +		uri_dp : eficonfig_create_device_path(dp_volume, current_path);
>   	if (!dp) {
>   		ret = EFI_OUT_OF_RESOURCES;
>   		goto out;
> @@ -1558,6 +1641,7 @@ static efi_status_t eficonfig_edit_boot_option(u16 *varname, struct eficonfig_bo
>   	ret = eficonfig_set_boot_option(varname, dp, dp_size, bo->description, tmp);
>   out:
>   	free(tmp);
> +	free(bo->uri);
>   	free(bo->optional_data);
>   	free(bo->description);
>   	free(bo->file_info.current_path);