[PATCH v5 08/14] efi_loader: net: set EFI bootdevice device path to HTTP when loaded from wget

[Heinrich Schuchardt]

On 25.11.24 01:58, Adriano Cordova wrote:
> Set the device path of the efi boot device to an HTTP device path
> (as formed by efi_dp_from_http) when the next boot stage is loaded
> using wget (i.e., when wget is used with wget_info.set_bootdev=1).
>
> When loaded from HTTP, the device path should account for it so that
> the next boot stage is aware (e.g. grub only loads its http stack if
> it itself was loaded from http, and it checks this from its device path).
>
> Signed-off-by: Adriano Cordova <adrianox at gmail.com>
> ---
> Changes in v5:
> - Add description of net_dp
> - Change a void** for an efi_device_path**
> (no changes since v2)
>
>   include/efi_loader.h             |  6 +++++
>   lib/efi_loader/efi_device_path.c |  2 +-
>   lib/efi_loader/efi_net.c         | 44 +++++++++++++++++++++++++++++++-
>   net/lwip/wget.c                  |  1 +
>   net/wget.c                       |  1 +
>   5 files changed, 52 insertions(+), 2 deletions(-)
>
> diff --git a/include/efi_loader.h b/include/efi_loader.h
> index 96b204dfc3..8e7f5becbb 100644
> --- a/include/efi_loader.h
> +++ b/include/efi_loader.h
> @@ -126,6 +126,10 @@ static inline void efi_set_bootdev(const char *dev, const char *devnr,
>   #endif
>
>   #if CONFIG_IS_ENABLED(NETDEVICES) && CONFIG_IS_ENABLED(EFI_LOADER)
> +/* Call this to update the current device path of the efi net device */
> +void efi_net_set_dp(const char *dev, const char *server);
> +/* Call this to get the current device path of the efi net device */
> +void efi_net_get_dp(struct efi_device_path **dp);
>   void efi_net_get_addr(struct efi_ipv4_address *ip,
>   		      struct efi_ipv4_address *mask,
>   		      struct efi_ipv4_address *gw);
> @@ -133,6 +137,8 @@ void efi_net_set_addr(struct efi_ipv4_address *ip,
>   		      struct efi_ipv4_address *mask,
>   		      struct efi_ipv4_address *gw);
>   #else
> +static inline void efi_net_set_dp(const char *dev, const char *server) { }
> +static inline void efi_net_get_dp(struct efi_device_path **dp) { }
>   static inline void efi_net_get_addr(struct efi_ipv4_address *ip,
>   				     struct efi_ipv4_address *mask,
>   				     struct efi_ipv4_address *gw) { }
> diff --git a/lib/efi_loader/efi_device_path.c b/lib/efi_loader/efi_device_path.c
> index 1e512bf0ea..be0b56d213 100644
> --- a/lib/efi_loader/efi_device_path.c
> +++ b/lib/efi_loader/efi_device_path.c
> @@ -1181,7 +1181,7 @@ efi_status_t efi_dp_from_name(const char *dev, const char *devnr,
>   		dp = efi_dp_from_mem(EFI_RESERVED_MEMORY_TYPE,
>   				     (uintptr_t)image_addr, image_size);
>   	} else if (IS_ENABLED(CONFIG_NETDEVICES) && !strcmp(dev, "Net")) {
> -		dp = efi_dp_from_eth();
> +		efi_net_get_dp(&dp);
>   	} else if (!strcmp(dev, "Uart")) {
>   		dp = efi_dp_from_uart();
>   	} else {
> diff --git a/lib/efi_loader/efi_net.c b/lib/efi_loader/efi_net.c
> index 3491d4c481..b448dcee51 100644
> --- a/lib/efi_loader/efi_net.c
> +++ b/lib/efi_loader/efi_net.c
> @@ -16,6 +16,7 @@
>    */
>
>   #include <efi_loader.h>
> +#include <dm.h>
>   #include <malloc.h>
>   #include <vsprintf.h>
>   #include <net.h>
> @@ -32,6 +33,13 @@ static int rx_packet_idx;
>   static int rx_packet_num;
>   static struct efi_net_obj *netobj;
>
> +/*
> + * The current network device path. This device path is updated when a new
> + * bootfile is downloaded from the network. If then the bootfile is loaded
> + * as an efi image, net_dp is passed as the device path of the loaded image.
> + */
> +static struct efi_device_path *net_dp;
> +
>   /*
>    * The notification function of this event is called in every timer cycle
>    * to check if a new network packet has been received.
> @@ -902,8 +910,10 @@ efi_status_t efi_net_register(void)
>   			     &netobj->net);
>   	if (r != EFI_SUCCESS)
>   		goto failure_to_add_protocol;
> +	if (!net_dp)
> +		efi_net_set_dp("Net", NULL);
>   	r = efi_add_protocol(&netobj->header, &efi_guid_device_path,
> -			     efi_dp_from_eth());
> +			     net_dp);
>   	if (r != EFI_SUCCESS)
>   		goto failure_to_add_protocol;
>   	r = efi_add_protocol(&netobj->header, &efi_pxe_base_code_protocol_guid,
> @@ -999,6 +1009,38 @@ out_of_resources:
>   	return EFI_OUT_OF_RESOURCES;
>   }
>
> +/**
> + * efi_net_set_dp() - set device path of efi net device
> + *
> + * This gets called to update the device path when a new boot
> + * file is dowloaded

%s/dowloaded/downloaded/

> + *
> + * @dev:	dev to set the device path from
> + * @server:	remote server address
> + */
> +void efi_net_set_dp(const char *dev, const char *server)
> +{

If this function is called repeatedly, we are leaking memory.
I could not find where you free net_dp after a transaction.
Maybe here is the right place if net_dp is not NULL.

> +	if (!strcmp(dev, "Net"))
> +		net_dp = efi_dp_from_eth();

efi_dp_from_eth() may fail and return NULL.

> +	else if (!strcmp(dev, "Http"))
> +		net_dp = efi_dp_from_http(server);

efi_dp_from_http() may fail and return NULL.

How will errors be handled?

> +}
> +
> +/**
> + * efi_net_get_dp() - get device path of efi net device
> + *
> + * Produce a copy of the current device path
> + *
> + * @dp:		copy of the current device path

copy of the current device path or NULL on error

> + */
> +void efi_net_get_dp(struct efi_device_path **dp)

struct efi_device_path *efi_net_get_dp(struct efi_device_path *dp)
would reduce dereferencing.

> +{
> +	if (!net_dp)
> +		efi_net_set_dp("Net", NULL);

efi_net_set_dp may fail. This needs to be handled. E.g. return NULL.

> +	if (dp && net_dp)
> +		*dp = efi_dp_dup(net_dp);
> +}

efi_dp_dup may return NULL if out of memory.

> +
>   /**
>    * efi_net_get_addr() - get IP address information
>    *
> diff --git a/net/lwip/wget.c b/net/lwip/wget.c
> index 3347a61e74..7bc7c6a2fe 100644
> --- a/net/lwip/wget.c
> +++ b/net/lwip/wget.c
> @@ -217,6 +217,7 @@ static void httpc_result_cb(void *arg, httpc_result_t httpc_result,
>   	print_size(rx_content_len / elapsed * 1000, "/s)\n");
>   	printf("Bytes transferred = %lu (%lx hex)\n", ctx->size, ctx->size);
>   	if (wget_info->set_bootdev) {
> +		efi_net_set_dp("Http", ctx->server_name);

Error handling is missing.

>   		efi_set_bootdev("Net", "", ctx->path, map_sysmem(ctx->saved_daddr, 0),
>   				rx_content_len);
>   	}
> diff --git a/net/wget.c b/net/wget.c
> index 4bb1590410..21d3b8050a 100644
> --- a/net/wget.c
> +++ b/net/wget.c
> @@ -442,6 +442,7 @@ static void wget_handler(uchar *pkt, u16 dport,
>   		net_set_state(wget_loop_state);
>   		wget_info->file_size = net_boot_file_size;
>   		if (wget_info->method == WGET_HTTP_METHOD_GET && wget_info->set_bootdev) {
> +			efi_net_set_dp("Http", NULL);

Error handling is missing.

Best regards

Heinrich

>   			efi_set_bootdev("Net", "", image_url,
>   					map_sysmem(image_load_addr, 0),
>   					net_boot_file_size);