Hi Michal,
On Wed, 7 Aug 2024 at 23:31, Michal Simek <michal.si...@amd.com> wrote:
>
>
>
> On 8/7/24 16:36, Simon Glass wrote:
> > Hi Prasad,
> >
> > On Tue, 6 Aug 2024 at 23:05, Kummari, Prasad <prasad.kumm...@amd.com> wrote:
> >>
> >> Hi Glass,
> >>
> >>> -----Original Message-----
> >>> From: Simon Glass <s...@chromium.org>
> >>> Sent: Wednesday, August 7, 2024 3:21 AM
> >>> To: Kummari, Prasad <prasad.kumm...@amd.com>
> >>> Cc: u-boot@lists.denx.de; git (AMD-Xilinx) <g...@amd.com>; Simek, Michal
> >>> <michal.si...@amd.com>; Abbarapu, Venkatesh
> >>> <venkatesh.abbar...@amd.com>; g...@xilinx.com;
> >>> ja...@amarulasolutions.com; n-fran...@ti.com; d-g...@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.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.
> >>>
> >>> 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
> >
> > Yes. There are many things which can overwrite memory, e.g. the mw
> > command. It is a boot loader so this is normal.
> >
> > What image are you loading here?
>
> In spi boot it can be Kernel/rootfs but at the end of day it doesn't really
> matter.
OK, in that case yes it should use lmb. That was the question I was
trying to understand.
>
> We have protection for srec, fs load, tftp and wget already.
>
> c6855195e4b4 ("loads: Block writes into LMB reserved areas of U-Boot")
> aa3c609e2be5 ("fs: prevent overwriting reserved memory")
> a156c47e39ad ("tftp: prevent overwriting reserved memory")
> 04592adbdb99 ("net: wget: prevent overwriting reserved memory")
>
> And this is just +1 patch to protect sf command that it doesn't touch reserved
> location.
> The same code should be used for other commands(nand, usb, etc) which loading
> block of data to memory because all of them shouldn't rewrite reserved memory.
>
> In connection to mw/mtest/etc command protection can be also done but not sure
> if this is useful because you normally not using them for booting.
Exactly.
I am hoping that we can pull SPI flash into bootstd...has anyone
looked at that? Are you using scripts or is there a special bootmeth?
Regards,
Simon
