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

[Michal Simek]

On 8/7/24 16:36, Simon Glass wrote:
> Hi Prasad,
> 
> On Tue, 6 Aug 2024 at 23:05, Kummari, Prasad <Prasad.Kummari at amd.com> wrote:
>>
>> 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
> 
> 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.

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.

Thanks,
Michal