| Message ID | 20250417132718.2023555-4-jerome.forissier@linaro.org |
|---|---|
| State | Accepted |
| Commit | e40c910e88a9a38ee1335ab1391dc2ee83993a65 |
| Headers | show |
| Series | NET_LWIP LMB fixes | expand |
On Thu, 17 Apr 2025 at 18:57, Jerome Forissier <jerome.forissier@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.forissier@linaro.org> > Suggested-by: Sughosh Ganu <sughosh.ganu@linaro.org> > Acked-by: Sughosh Ganu <sughosh.ganu@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.ganu@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 >
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,