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

[Kummari, Prasad]

Hi Sughosh,

> -----Original Message-----
> From: Sughosh Ganu <sughosh.ganu at linaro.org>
> Sent: Monday, September 9, 2024 3:48 PM
> To: Kummari, Prasad <Prasad.Kummari at amd.com>
> Cc: u-boot at lists.denx.de; git (AMD-Xilinx) <git at amd.com>; Simek, Michal
> <michal.simek at amd.com>; Abbarapu, Venkatesh
> <venkatesh.abbarapu at amd.com>; Begari, Padmarao
> <Padmarao.Begari at amd.com>; git at xilinx.com;
> jagan at amarulasolutions.com; n-francis at ti.com; d-gole at ti.com
> Subject: Re: [PATCH v2] cmd: sf: prevent overwriting the reserved memory
> 
> Caution: This message originated from an External Source. Use proper
> caution when opening attachments, clicking links, or responding.
> 
> 
> 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.

[Prasad]:  Apologies for the incorrect patch sent, I missed updating the LMB APIs in this version. I will send v3 with the necessary changes included.

> 
> -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
> >