On Thu, 17 Apr 2025 at 18:57, Jerome Forissier
<jerome.foriss...@linaro.org> 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>
> Acked-by: Sughosh Ganu <sughosh.g...@linaro.org>
> ---
Tested that the LMB checks are performed and wget/tftp commands do not
overwrite on an existing LMB reservation.
Tested-by: Sughosh Ganu <sughosh.g...@linaro.org>
-sughosh
>
> (no changes since v4)
>
> Changes in v4:
> - The 'silent' boolean in stored in struct wget_http_info (same as NET)
>
> Changes in v3:
> - store_block(): add Sphinx-like documentation
> - store_block(): do not print to the console if ctx->silent
>
> Changes in v2:
> - httpc_recv_cb(): add a call to altcp_abort(). Otherwise the transfer
> continues and we try to write later blocks which makes no sense if one
> has been rejected already. Thanks Sughosh G. for testing and reporting.
>
> net/lwip/wget.c | 64 ++++++++++++++++++++++++++++++++++++++++++-------
> 1 file changed, 55 insertions(+), 9 deletions(-)
>
> diff --git a/net/lwip/wget.c b/net/lwip/wget.c
> index 63583e4c6e7..4ec00de96b2 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/errno.h>
> @@ -202,11 +203,58 @@ static int parse_legacy_arg(char *arg, char *nurl,
> size_t rem)
> return 0;
> }
>
> +/**
> + * store_block() - copy received data
> + *
> + * This function is called by the receive callback to copy a block of data
> + * into its final location (ctx->daddr). Before doing so, it checks if the
> copy
> + * is allowed.
> + *
> + * @ctx: the context for the current transfer
> + * @src: the data received from the TCP stack
> + * @len: the length of the data
> + */
> +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)) {
> + if (!wget_info->silent) {
> + printf("\nwget error: ");
> + printf("trying to overwrite reserved
> memory\n");
> + }
> + 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) {
> + if (!wget_info->silent)
> + printf("#");
> + 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;
> @@ -215,19 +263,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 (!wget_info->silent &&
> - 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,
> --
> 2.43.0
>
