On Thu, 17 Apr 2025 at 18:57, Jerome Forissier
<jerome.foriss...@linaro.org> wrote:
>
> Legacy NET tftp 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 v3)
>
> Changes in v3:
> - store_block(): add Sphinx-like documentation
>
> net/lwip/tftp.c | 56 +++++++++++++++++++++++++++++++++++++++----------
> 1 file changed, 45 insertions(+), 11 deletions(-)
>
> diff --git a/net/lwip/tftp.c b/net/lwip/tftp.c
> index 123d66b5dba..c7f1420ca0f 100644
> --- a/net/lwip/tftp.c
> +++ b/net/lwip/tftp.c
> @@ -8,6 +8,7 @@
> #include <efi_loader.h>
> #include <image.h>
> #include <linux/delay.h>
> +#include <linux/kconfig.h>
> #include <lwip/apps/tftp_client.h>
> #include <lwip/timeouts.h>
> #include <mapmem.h>
> @@ -31,6 +32,47 @@ struct tftp_ctx {
> enum done_state done;
> };
>
> +/**
> + * 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 tftp_ctx *ctx, void *src, u16_t len)
> +{
> + ulong store_addr = ctx->daddr;
> + void *ptr;
> +
> + if (CONFIG_IS_ENABLED(LMB)) {
> + if (store_addr + len < store_addr ||
> + lmb_read_check(store_addr, len)) {
> + puts("\nTFTP error: ");
> + puts("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;
> + ctx->block_count++;
> + if (ctx->block_count % 10 == 0) {
> + putc('#');
> + if (ctx->block_count % (65 * 10) == 0)
> + puts("\n\t ");
> + }
> +
> + return 0;
> +}
> +
> static void *tftp_open(const char *fname, const char *mode, u8_t is_write)
> {
> return NULL;
> @@ -71,17 +113,9 @@ static int tftp_write(void *handle, struct pbuf *p)
> struct tftp_ctx *ctx = handle;
> struct pbuf *q;
>
> - for (q = p; q; q = q->next) {
> - memcpy((void *)ctx->daddr, q->payload, q->len);
> - ctx->daddr += q->len;
> - ctx->size += q->len;
> - ctx->block_count++;
> - if (ctx->block_count % 10 == 0) {
> - putc('#');
> - if (ctx->block_count % (65 * 10) == 0)
> - puts("\n\t ");
> - }
> - }
> + for (q = p; q; q = q->next)
> + if (store_block(ctx, q->payload, q->len) < 0)
> + return -1;
>
> return 0;
> }
> --
> 2.43.0
>
