linux-kernel - Re: [PATCH 1/3] loongarch: Call arch_mem_init() before platform

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Wed, 14 Feb 2024 13:31:28 -0800
From: Oreoluwa Babatunde <quic_obabatun@...cinc.com>
To: Huacai Chen <chenhuacai@...nel.org>
CC: <jonas@...thpole.se>, <stefan.kristiansson@...nalahti.fi>,
        <shorne@...il.com>, <ysato@...rs.sourceforge.jp>, <dalias@...c.org>,
        <glaubitz@...sik.fu-berlin.de>, <robh+dt@...nel.org>,
        <frowand.list@...il.com>, <linux-openrisc@...r.kernel.org>,
        <loongarch@...ts.linux.dev>, <linux-sh@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>, <devicetree@...r.kernel.org>,
        <kernel@...cinc.com>
Subject: Re: [PATCH 1/3] loongarch: Call arch_mem_init() before
 platform_init() in the init sequence

On 2/14/2024 5:03 AM, Huacai Chen wrote:
> Hi, Oreoluwa,
>
> On Sat, Feb 10, 2024 at 8:29 AM Oreoluwa Babatunde
> <quic_obabatun@...cinc.com> wrote:
>> The platform_init() function which is called during device bootup
>> contains a few calls to memblock_alloc().
>> This is an issue because these allocations are done before reserved
>> memory regions are set aside in arch_mem_init().
>> This means that there is a possibility for memblock to allocate memory
>> from any of the reserved memory regions.
>>
>> Hence, move the call to arch_mem_init() to be earlier in the init
>> sequence so that all reserved memory is set aside before any allocations
>> are made with memblock.
>>
>> Signed-off-by: Oreoluwa Babatunde <quic_obabatun@...cinc.com>
>> ---
>>  arch/loongarch/kernel/setup.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/loongarch/kernel/setup.c b/arch/loongarch/kernel/setup.c
>> index edf2bba..66c307c 100644
>> --- a/arch/loongarch/kernel/setup.c
>> +++ b/arch/loongarch/kernel/setup.c
>> @@ -597,8 +597,8 @@ void __init setup_arch(char **cmdline_p)
>>         parse_early_param();
>>         reserve_initrd_mem();
>>
>> -       platform_init();
>>         arch_mem_init(cmdline_p);
>> +       platform_init();
> Thank you for your patch, but I think we cannot simply exchange their
> order. If I'm right, you try to move all memblock_reserve() as early
> as possible, but both arch_mem_init() and platform_init() call
> memblock_reserve(), we should do a complete refactor for this. And
> since it works with the existing order, we can simply keep it as is
> now.
>
> Huacai
Hi Huacai,

Thank you for your response!

I'm not trying to move all memblock_reserve() to be as early as possible,
I'm trying to move the call to early_init_fdt_scan_reserved_mem() to be
as early as possible. This is the function that is used to set aside all the
reserved memory regions that are meant for certain devices/drivers.

The reserved memory regions I am referring to are explicitly defined in
the DT. These regions are set aside so that the system will have either
limited access or no access to them at all.
Some of these regions are also defined with a property called no-map
which tells the system not to create a memory mapping for them.
https://github.com/devicetree-org/dt-schema/blob/main/dtschema/schemas/reserved-memory/reserved-memory.yaml#L79

Hence, setting aside these memory regions should take priority and should
be done first before any memblock allocations are done so that the system
does not  unknowingly allocate memory from a region that is meant to be
reserved for a device/driver.

Eg:
    unflatten_and_copy_device_tree() eventually calls memblock_alloc():
    https://elixir.bootlin.com/linux/latest/source/drivers/of/fdt.c#L1264

    Since unflatten_and_copy_device_tree() is called in platform_init(), this
    allocation is done before we are able to set aside any of the reserved
    memory regions from the DT which is supposed to be done by
    early_init_fdt_scan_reserved_mem() in the arch_mem_init() function.

    Hence, it is possible for unflatten_and_copy_device_tree() to allocate
    memory from a region that is meant to be set aside for a device/driver
    without the system knowing.

This can create problems for a device/driver if a region of memory that was
supposed to be set aside for it ends up being allocated for another use case
by memblock_alloc*().

Regards,

Oreoluwa