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

[Kummari, Prasad]

Hi Glass,

> -----Original Message-----
> From: Simon Glass <sjg at chromium.org>
> Sent: Wednesday, August 7, 2024 3:21 AM
> 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>; git at xilinx.com;
> jagan at amarulasolutions.com; n-francis at ti.com; d-gole at ti.com
> Subject: Re: [PATCH] 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.
> 
> 
> Hi Prasad,
> 
> On Tue, 6 Aug 2024 at 06:08, 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.
> 
> The SPI flash may be used to load other things, not just an OS. What is your
> use case or problem here?

[Prasad]:  We have observed that SF command can overwrite the reserved area without throwing any errors or warnings.
 This issue was noticed when the TF-A area is reserved in the Device Tree at address 0xf000000. The sf command is
 corrupting the reserved area,  and U-Boot relocation address too.

EX: TF-A reserved at ddr address 0xf000000
 
      Versal NET> sf read 0x0f000000 0x0 0x100     ----> Overwriting reserved area.
      device 0 offset 0x0, size 0x100
      SF: 256 bytes @ 0x0 Read: OK

     U-boot relocation address relocaddr   = 0x000000007fec2000

      Versal NET> sf write 0x0000000077ec2000 0x0 0x100   --> Overwriting reserved area.
      device 0 offset 0x0, size 0x100
      SF: 256 bytes @ 0x0 Written: OK

> 
> Regards,
> Simon
> 
> 
> >
> > Signed-off-by: Prasad Kummari <prasad.kummari at amd.com>
> > ---
> > 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);
> > +
> > +       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
> >