[v3,08/15] efi_loader: device_path: add support for HTTP device path

Add efi_dp_from_http to form a device path from HTTP. The
device path is the concatenation of the device path returned
by efi_dp_from_ipv4 together with an URI node and an END node.

Signed-off-by: Adriano Cordova <adrianox@gmail.com>
---

Changes in v3:
- Moved argument checks in efi_dp_from_http to the beginning of the function

 include/efi_loader.h             |  1 +
 lib/efi_loader/efi_device_path.c | 55 ++++++++++++++++++++++++++++++++
 2 files changed, 56 insertions(+)

On Mon, 11 Nov 2024 at 23:10, Adriano Cordova <adrianox@gmail.com> wrote:
>
> Add efi_dp_from_http to form a device path from HTTP. The
> device path is the concatenation of the device path returned
> by efi_dp_from_ipv4 together with an URI node and an END node.
>
> Signed-off-by: Adriano Cordova <adrianox@gmail.com>
> ---
>
> Changes in v3:
> - Moved argument checks in efi_dp_from_http to the beginning of the function
>
>  include/efi_loader.h             |  1 +
>  lib/efi_loader/efi_device_path.c | 55 ++++++++++++++++++++++++++++++++
>  2 files changed, 56 insertions(+)
>
> diff --git a/include/efi_loader.h b/include/efi_loader.h
> index 612bc42816..96b204dfc3 100644
> --- a/include/efi_loader.h
> +++ b/include/efi_loader.h
> @@ -872,6 +872,7 @@ struct efi_device_path *efi_dp_part_node(struct blk_desc *desc, int part);
>  struct efi_device_path *efi_dp_from_file(const struct efi_device_path *dp,
>                                          const char *path);
>  struct efi_device_path *efi_dp_from_eth(void);
> +struct efi_device_path *efi_dp_from_http(const char *server);
>  struct efi_device_path *efi_dp_from_mem(uint32_t mem_type,
>                                         uint64_t start_address,
>                                         size_t size);
> diff --git a/lib/efi_loader/efi_device_path.c b/lib/efi_loader/efi_device_path.c
> index 82a45da1fa..158f08b9e5 100644
> --- a/lib/efi_loader/efi_device_path.c
> +++ b/lib/efi_loader/efi_device_path.c
> @@ -1012,6 +1012,61 @@ struct efi_device_path *efi_dp_from_ipv4(struct efi_ipv4_address *ip,
>         return dp2;
>  }
>
> +struct efi_device_path *efi_dp_from_http(const char *server)
> +{
> +       struct efi_device_path *dp1, *dp2;
> +       struct efi_device_path_uri *uridp;
> +       efi_uintn_t uridp_len;
> +       char *pos;
> +       char tmp[128];
> +       struct efi_ipv4_address ip;
> +       struct efi_ipv4_address mask;
> +
> +       if ((server && strlen("http://") + strlen(server) + 1  > 128)

sizeof (tmp) instead of 128

> +           (!server && IS_ENABLED(CONFIG_NET_LWIP)))
> +               return NULL;
> +
> +       efi_net_get_addr(&ip, &mask, NULL);
> +
> +       dp1 = efi_dp_from_ipv4(&ip, &mask, NULL);
> +
> +       strcpy(tmp, "http://");

I've sent this a while back which is close to merging [0]. I am ok
with merging http support first, but if it's not too much of a pain
can you rebase on top of that and add https:// as well?

> +
> +#if IS_ENABLED(CONFIG_NET_LWIP)
> +       if (server) {
> +               memcpy(tmp + strlen("http://"), server, strlen(server) + 1);
> +       }
> +#else
> +       if (server) {
> +               memcpy(tmp + strlen("http://"), server, strlen(server) + 1);
> +       } else {
> +               ip_to_string(net_server_ip, tmp + strlen("http://"));
> +       }
> +#endif
> +

I think it's easier to read if you rewrite this as
if (server)
    memcpy(tmp + strlen("http://"), server, strlen(server) + 1);
else
     if (!IS_ENABLED(CONFIG_NET_LWIP)
         ip_to_string(net_server_ip, tmp + strlen("http://"));

> +       uridp_len = sizeof(struct efi_device_path) + strlen(tmp) + 1;
> +       uridp = efi_alloc(uridp_len + sizeof(END));
> +       if (!uridp) {
> +               log_err("Out of memory\n");
> +               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;
> +       debug("device path: setting uri device path to %s\n", tmp);
> +       memcpy(uridp->uri, tmp, strlen(tmp) + 1);
> +
> +       pos = (char *)uridp + uridp_len;
> +       memcpy(pos, &END, sizeof(END));
> +
> +       dp2 = efi_dp_concat(dp1, (const struct efi_device_path *)uridp, 0);
> +
> +       efi_free_pool(uridp);
> +       efi_free_pool(dp1);
> +
> +       return dp2;
> +}
> +
>  /* Construct a device-path for memory-mapped image */
>  struct efi_device_path *efi_dp_from_mem(uint32_t memory_type,
>                                         uint64_t start_address,
> --
> 2.43.0
>

[0] https://lore.kernel.org/u-boot/20241110083017.367565-1-ilias.apalodimas@linaro.org/

Thanks
/Ilias

El mié, 13 nov 2024 a las 5:16, Ilias Apalodimas (<
ilias.apalodimas@linaro.org>) escribió:

> On Mon, 11 Nov 2024 at 23:10, Adriano Cordova <adrianox@gmail.com> wrote:
> >
> > Add efi_dp_from_http to form a device path from HTTP. The
> > device path is the concatenation of the device path returned
> > by efi_dp_from_ipv4 together with an URI node and an END node.
> >
> > Signed-off-by: Adriano Cordova <adrianox@gmail.com>
> > ---
> >
> > Changes in v3:
> > - Moved argument checks in efi_dp_from_http to the beginning of the
> function
> >
> >  include/efi_loader.h             |  1 +
> >  lib/efi_loader/efi_device_path.c | 55 ++++++++++++++++++++++++++++++++
> >  2 files changed, 56 insertions(+)
> >
> > diff --git a/include/efi_loader.h b/include/efi_loader.h
> > index 612bc42816..96b204dfc3 100644
> > --- a/include/efi_loader.h
> > +++ b/include/efi_loader.h
> > @@ -872,6 +872,7 @@ struct efi_device_path *efi_dp_part_node(struct
> blk_desc *desc, int part);
> >  struct efi_device_path *efi_dp_from_file(const struct efi_device_path
> *dp,
> >                                          const char *path);
> >  struct efi_device_path *efi_dp_from_eth(void);
> > +struct efi_device_path *efi_dp_from_http(const char *server);
> >  struct efi_device_path *efi_dp_from_mem(uint32_t mem_type,
> >                                         uint64_t start_address,
> >                                         size_t size);
> > diff --git a/lib/efi_loader/efi_device_path.c
> b/lib/efi_loader/efi_device_path.c
> > index 82a45da1fa..158f08b9e5 100644
> > --- a/lib/efi_loader/efi_device_path.c
> > +++ b/lib/efi_loader/efi_device_path.c
> > @@ -1012,6 +1012,61 @@ struct efi_device_path *efi_dp_from_ipv4(struct
> efi_ipv4_address *ip,
> >         return dp2;
> >  }
> >
> > +struct efi_device_path *efi_dp_from_http(const char *server)
> > +{
> > +       struct efi_device_path *dp1, *dp2;
> > +       struct efi_device_path_uri *uridp;
> > +       efi_uintn_t uridp_len;
> > +       char *pos;
> > +       char tmp[128];
> > +       struct efi_ipv4_address ip;
> > +       struct efi_ipv4_address mask;
> > +
> > +       if ((server && strlen("http://") + strlen(server) + 1  > 128)
>
> sizeof (tmp) instead of 128
>
> > +           (!server && IS_ENABLED(CONFIG_NET_LWIP)))
> > +               return NULL;
> > +
> > +       efi_net_get_addr(&ip, &mask, NULL);
> > +
> > +       dp1 = efi_dp_from_ipv4(&ip, &mask, NULL);
> > +
> > +       strcpy(tmp, "http://");
>
> I've sent this a while back which is close to merging [0]. I am ok
> with merging http support first, but if it's not too much of a pain
> can you rebase on top of that and add https:// as well?
>

For that I'd have to change the wget patches too. I will try to add it in
the next version,
otherwise I can send a separated patch.


> > +
> > +#if IS_ENABLED(CONFIG_NET_LWIP)
> > +       if (server) {
> > +               memcpy(tmp + strlen("http://"), server, strlen(server)
> + 1);
> > +       }
> > +#else
> > +       if (server) {
> > +               memcpy(tmp + strlen("http://"), server, strlen(server)
> + 1);
> > +       } else {
> > +               ip_to_string(net_server_ip, tmp + strlen("http://"));
> > +       }
> > +#endif
> > +
>
> I think it's easier to read if you rewrite this as
> if (server)
>     memcpy(tmp + strlen("http://"), server, strlen(server) + 1);
> else
>      if (!IS_ENABLED(CONFIG_NET_LWIP)
>          ip_to_string(net_server_ip, tmp + strlen("http://"));
>
> I agree, but then there is an empty else { } , not sure if that would
generate a warning.

> +       uridp_len = sizeof(struct efi_device_path) + strlen(tmp) + 1;
> > +       uridp = efi_alloc(uridp_len + sizeof(END));
> > +       if (!uridp) {
> > +               log_err("Out of memory\n");
> > +               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;
> > +       debug("device path: setting uri device path to %s\n", tmp);
> > +       memcpy(uridp->uri, tmp, strlen(tmp) + 1);
> > +
> > +       pos = (char *)uridp + uridp_len;
> > +       memcpy(pos, &END, sizeof(END));
> > +
> > +       dp2 = efi_dp_concat(dp1, (const struct efi_device_path *)uridp,
> 0);
> > +
> > +       efi_free_pool(uridp);
> > +       efi_free_pool(dp1);
> > +
> > +       return dp2;
> > +}
> > +
> >  /* Construct a device-path for memory-mapped image */
> >  struct efi_device_path *efi_dp_from_mem(uint32_t memory_type,
> >                                         uint64_t start_address,
> > --
> > 2.43.0
> >
>
> [0]
> https://lore.kernel.org/u-boot/20241110083017.367565-1-ilias.apalodimas@linaro.org/
>
> Thanks
> /Ilias
>