On Mon, 9 Sept 2024 at 15:15, Prasad Kummari <prasad.kumm...@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.kumm...@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
>
