[PATCH v2] cmd: sf: prevent overwriting the reserved memory

[Sughosh Ganu]

On Mon, 9 Sept 2024 at 15:15, Prasad Kummari <prasad.kummari at amd.com> wrote:
>
> Added LMB API to prevent SF command from overwriting reserved
> memory areas. The current SPI code does not use LMB APIs for
> loading data into memory addresses. To resolve this, LMB APIs
> were added to check the load address of an SF command and ensure it
> does not overwrite reserved memory addresses. Similar checks are
> used in TFTP, serial load, and boot code to prevent overwriting
> reserved memory.
>
> Signed-off-by: Prasad Kummari <prasad.kummari at amd.com>
> ---
>
> Changes in V2:
> - Rebased the code changes on top of the next branch.
>
> UT:
> Tested on Versal NET board
>
> Versal NET> sf read 0xf000000 0x0 0x100
> device 0 offset 0x0, size 0x100
> ERROR: trying to overwrite reserved memory...
>
> Versal NET> sf read 0x79ea9000 0x 0x100
> device 0 offset 0x0, size 0x100
> ERROR: trying to overwrite reserved memory...
>
> Versal NET> sf write 0x79ea9000 0x 0x100
> device 0 offset 0x0, size 0x100
> ERROR: trying to overwrite reserved memory...
>
> Versal NET> sf write 0x79eaf000 0x20000 0x100
> device 0 offset 0x20000, size 0x100
> ERROR: trying to overwrite reserved memory...
>
>
>  cmd/sf.c | 38 +++++++++++++++++++++++++++++++++++++-
>  1 file changed, 37 insertions(+), 1 deletion(-)
>
> diff --git a/cmd/sf.c b/cmd/sf.c
> index f43a2e08b3..d2ce1af8a0 100644
> --- a/cmd/sf.c
> +++ b/cmd/sf.c
> @@ -10,6 +10,7 @@
>  #include <div64.h>
>  #include <dm.h>
>  #include <log.h>
> +#include <lmb.h>
>  #include <malloc.h>
>  #include <mapmem.h>
>  #include <spi.h>
> @@ -272,6 +273,35 @@ static int spi_flash_update(struct spi_flash *flash, u32 offset,
>         return 0;
>  }
>
> +#ifdef CONFIG_LMB
> +static int do_spi_read_lmb_check(ulong start_addr, loff_t len)
> +{
> +       struct lmb lmb;
> +       phys_size_t max_size;
> +       ulong end_addr;
> +
> +       lmb_init_and_reserve(&lmb, gd->bd, (void *)gd->fdt_blob);
> +       lmb_dump_all(&lmb);

Are you sure this has been rebased and tested on top of the next
branch. The lmb_init_and_reserve() function has been removed as part
of the LMB rework series. And the other API's are incorrect. Also,
please check the fs_read_lmb_check() function to see how this is to be
implemented. There is a call to the lmb_alloc_addr() function which
seems to be missing here. And lastly, it would help if we can refactor
this code so that all modules can reuse it. Thanks.

-sughosh

> +
> +       max_size = lmb_get_free_size(&lmb, start_addr);
> +       if (!max_size) {
> +               printf("ERROR: trying to overwrite reserved memory...\n");
> +               return CMD_RET_FAILURE;
> +       }
> +
> +       end_addr = start_addr + max_size;
> +       if (!end_addr)
> +               end_addr = ULONG_MAX;
> +
> +       if ((start_addr + len) > end_addr) {
> +               printf("ERROR: trying to overwrite reserved memory...\n");
> +               return CMD_RET_FAILURE;
> +       }
> +
> +       return 0;
> +}
> +#endif
> +
>  static int do_spi_flash_read_write(int argc, char *const argv[])
>  {
>         unsigned long addr;
> @@ -315,7 +345,13 @@ static int do_spi_flash_read_write(int argc, char *const argv[])
>                 ret = spi_flash_update(flash, offset, len, buf);
>         } else if (strncmp(argv[0], "read", 4) == 0 ||
>                         strncmp(argv[0], "write", 5) == 0) {
> -               int read;
> +               int read, ret;
> +
> +#ifdef CONFIG_LMB
> +               ret = do_spi_read_lmb_check(addr, len);
> +               if (ret)
> +                       return ret;
> +#endif
>
>                 read = strncmp(argv[0], "read", 4) == 0;
>                 if (read)
> --
> 2.25.1
>