Re: [PATCH v2 1/2] net-lwip: wget: add LMB and buffer checks

[Heinrich Schuchardt]

On 09.04.25 14:20, Jerome Forissier wrote:

Legacy NET wget invokes a store_block() function which performs buffer
validation (LMB, address wrapping). Do the same with NET_LWIP.

Signed-off-by: Jerome Forissier <jerome.foriss...@linaro.org>
Suggested-by: Sughosh Ganu <sughosh.g...@linaro.org>
---
  net/lwip/wget.c | 49 +++++++++++++++++++++++++++++++++++++++++--------
  1 file changed, 41 insertions(+), 8 deletions(-)

diff --git a/net/lwip/wget.c b/net/lwip/wget.c
index 14f27d42998..746c8164d66 100644
--- a/net/lwip/wget.c
+++ b/net/lwip/wget.c
@@ -6,6 +6,7 @@
  #include <display_options.h>
  #include <efi_loader.h>
  #include <image.h>
+#include <linux/kconfig.h>
  #include <lwip/apps/http_client.h>
  #include "lwip/altcp_tls.h"
  #include <lwip/timeouts.h>
@@ -201,11 +202,44 @@ static int parse_legacy_arg(char *arg, char *nurl, size_t 
rem)
        return 0;
  }

Please, add Sphinx style descriptions to all functions.

+static int store_block(struct wget_ctx *ctx, void *src, u16_t len)
+{
+       ulong store_addr = ctx->daddr;
+       uchar *ptr;
+
+       /* Avoid overflow */
+       if (wget_info->buffer_size && wget_info->buffer_size < ctx->size + len)
+               return -1;
+
+       if (CONFIG_IS_ENABLED(LMB) && wget_info->set_bootdev) {
+               if (store_addr + len < store_addr ||
+                   lmb_read_check(store_addr, len)) {
+                       printf("\nwget error: ");
+                       printf("trying to overwrite reserved memory...\n");

Output while an EFI application is consuming the HTTP protocol should be
avoided as it is not compatible with the EFI console.

If you want console output in the wget command, please, implement it
there and not in a library function called by efi_net_do_request().

+                       return -1;
+               }
+       }
+
+       ptr = map_sysmem(store_addr, len);
+       memcpy(ptr, src, len);
+       unmap_sysmem(ptr);
+
+       ctx->daddr += len;
+       ctx->size += len;
+       if (ctx->size - ctx->prevsize > PROGRESS_PRINT_STEP_BYTES) {
+               printf("#");

ditto

Best regards

Heinrich

+               ctx->prevsize = ctx->size;
+       }
+
+       return 0;
+}
+
  static err_t httpc_recv_cb(void *arg, struct altcp_pcb *pcb, struct pbuf 
*pbuf,
                           err_t err)
  {
        struct wget_ctx *ctx = arg;
        struct pbuf *buf;
+       err_t ret;

        if (!pbuf)
                return ERR_BUF;
@@ -214,18 +248,17 @@ static err_t httpc_recv_cb(void *arg, struct altcp_pcb 
*pcb, struct pbuf *pbuf,
                ctx->start_time = get_timer(0);

        for (buf = pbuf; buf; buf = buf->next) {
-               memcpy((void *)ctx->daddr, buf->payload, buf->len);
-               ctx->daddr += buf->len;
-               ctx->size += buf->len;
-               if (ctx->size - ctx->prevsize > PROGRESS_PRINT_STEP_BYTES) {
-                       printf("#");
-                       ctx->prevsize = ctx->size;
+               if (store_block(ctx, buf->payload, buf->len) < 0) {
+                       altcp_abort(pcb);
+                       ret = ERR_BUF;
+                       goto out;
                }
        }
-
        altcp_recved(pcb, pbuf->tot_len);
+       ret = ERR_OK;
+out:
        pbuf_free(pbuf);
-       return ERR_OK;
+       return ret;
  }

  static void httpc_result_cb(void *arg, httpc_result_t httpc_result,