[U-Boot] [PATCH 3/4] fs: prevent overwriting reserved memory

[Simon Goldschmidt]

On 13.11.2018 20:42, Heinrich Schuchardt wrote:
> On 11/13/18 6:47 AM, Simon Goldschmidt wrote:
>> On Tue, Nov 13, 2018 at 3:23 AM Fabio Estevam <festevam at gmail.com> wrote:
>>> Hi Simon,
>>>
>>> On Mon, Nov 12, 2018 at 7:25 PM Simon Goldschmidt
>>> <simon.k.r.goldschmidt at gmail.com> wrote:
>>>
>>>> diff --git a/fs/fs.c b/fs/fs.c
>>>> index adae98d021..4baf6b1c39 100644
>>>> --- a/fs/fs.c
>>>> +++ b/fs/fs.c
>>>> @@ -428,13 +428,57 @@ int fs_size(const char *filename, loff_t *size)
>>>>          return ret;
>>>>   }
>>>>
>>>> -int fs_read(const char *filename, ulong addr, loff_t offset, loff_t len,
>>>> -           loff_t *actread)
>>>> +#ifdef CONFIG_LMB
>>> Unrelated to your series, but I was wondering if we could get rid of
>>> the CONFIG_LMB option.
>>>
>>> As far as I can see all the architectures define it, the only
>>> exception being arch/sh.
>>>
>>> If you agree I can send a patch after your series gets applied that
>>> removes CONFIG_LMB.
>> Sure, that would clean things up.
>>
>> Simon
>>
> NAK
>
> This patch-series does not provide what is needed. With
> odroid-c2_defconfig I get
>
> fdt list /reserved-memory/secmon at 10000000
> reserved-memory {
>          secmon at 10000000 {
>                  reg = <0x00000000 0x10000000 0x00000000 0x00200000>;
>                  no-map;
>          };
> };
>
> => load mmc 0:1 0x10000000 dtb
> 22925 bytes read in 8 ms (2.7 MiB/s)
>
> So now I have successfully overwritten the secure monitor. Urrgh.
>
> As you have observed load is still writing into a memory area that is
> reserved by the device-tree.
>
> Please, iterate over the device tree to ensure that nothing is loaded
> into a reserved memory area. Do not expect board files to do anything
> but create the reserve-memory entry in the device tree.

First of all, thanks for testing. I must admit I haven't tested this 
case, I just had the impression the existing function 
'boot_fdt_add_mem_rsv_regions()' (in image-fdt.c) would do that. I'll 
have to dig into why it doesn't.

Or do you have CONFIG_OF_LIBFDT disabled?

In any case, it *was* my intention to include the dts memory 
reservation! I'll make sure I test that for a v2.

Simon