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

[Michal Simek <michal.simek@amd.com>]

Hi Tom,

On 8/8/24 17:46, Tom Rini wrote:
> On Thu, Aug 08, 2024 at 01:18:44PM +0200, Michal Simek wrote:
>>
>>
>> On 8/8/24 08:22, Sughosh Ganu wrote:
>>> On Thu, 8 Aug 2024 at 11:05, Michal Simek <michal.simek@amd.com> wrote:
>>>>
>>>>
>>>>
>>>> On 8/7/24 23:12, Tom Rini wrote:
>>>>> On Tue, Aug 06, 2024 at 05:37:00PM +0530, Prasad Kummari 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>
>>>>>
>>>>> This is a much more generic issue that should be looked in to with the
>>>>> LMB rewrite that Sughosh is working on.
>>>>
>>>> yes. And is it going to be the part of his series?
>>>> I expect that if he accepts this will be done on the top of it and there is
>>>> likely no reason to wait.
>>>
>>> This change would be needed, but in a different form. The patch, since
>>> based on the current master branch, is assuming a local lmb memory
>>> map. My series is doing away with that, and so we will no longer have
>>> the lmb_init_and_reserve() API, for example. I would suggest that the
>>> patch be put out either on top of my patches, or ideally, once the lmb
>>> patches get merged.
>>
>> I care that we can't overwrite reserved memory by any of load commands.
>> Better to be fixed earlier rather than later but up to Tom to decide.
>>  From my perspective this is incorrect behavior which is fixing issue and
>> likely this can go to 2024.10 version.
>> Your LMB series is likely going to target 2025.01.
> 
> So, from my point of view, this is a longstanding issue that I get why
> people are concerned, but I think it's missing a bigger point. For
> network loads, OK, no one needs physical access to do something
> malicious, so yes, it's important we check there every time. For
> filesystem loads? There's far far too many production devices using SD
> cards, so yes, a malicious actor needs physical access, but not much.
> For flash (SPI or NAND), at that point why doesn't the malicious actor
> just use "mw" instead? The device is in their position if they're able
> to hook up probes/etc.
> 
> So my current thought process is that yes, fixing SPI and NAND and all
> of the other forms of reading (outside of "cp") need to be fixed, as a
> follow-up series to what Sughosh is doing. And then reminding people
> that CMD_MEMORY is dangerous and perhaps think about splitting mw/etc
> out from "cmp/base/loop" so that CMD_MEMORY can be disabled for boards
> that want a more secure feel.

Pretty much new Kconfig option for production system should disable it.

> 
> I'm open to being convinced I'm wrong and this is a serious problem to
> address now, not later, however.

I have no problem with it if all of these go to 2025.01 version.

Thanks,
Michal