| Message ID | 20240909094509.1481038-1-prasad.kummari@amd.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | [v2] cmd: sf: prevent overwriting the reserved memory | expand |
On Mon, 9 Sept 2024 at 15:15, Prasad Kummari <prasad.kummari@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@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 >
Hi Sughosh, > -----Original Message----- > From: Sughosh Ganu <sughosh.ganu@linaro.org> > Sent: Monday, September 9, 2024 3:48 PM > To: Kummari, Prasad <Prasad.Kummari@amd.com> > Cc: u-boot@lists.denx.de; git (AMD-Xilinx) <git@amd.com>; Simek, Michal > <michal.simek@amd.com>; Abbarapu, Venkatesh > <venkatesh.abbarapu@amd.com>; Begari, Padmarao > <Padmarao.Begari@amd.com>; git@xilinx.com; > jagan@amarulasolutions.com; n-francis@ti.com; d-gole@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@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@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 > >
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)
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@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(-)