linux-kernel - Re: [PATCH v4] of/fdt: Don't worry about non-memory region overlap for no-map

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] [day] [month] [year] [list]

Message-ID: <YdjNhIs9/GH3YJ0P@robh.at.kernel.org>
Date:   Fri, 7 Jan 2022 17:32:20 -0600
From:   Rob Herring <robh@...nel.org>
To:     Stephen Boyd <swboyd@...omium.org>
Cc:     devicetree@...r.kernel.org, Quentin Perret <qperret@...gle.com>,
        Rob Herring <robh+dt@...nel.org>,
        Jan Kiszka <jan.kiszka@...mens.com>,
        linux-kernel@...r.kernel.org,
        Nicolas Boichat <drinkcat@...omium.org>,
        Mike Rapoport <rppt@...nel.org>,
        Douglas Anderson <dianders@...omium.org>
Subject: Re: [PATCH v4] of/fdt: Don't worry about non-memory region overlap
 for no-map

On Fri, 07 Jan 2022 11:42:32 -0800, Stephen Boyd wrote:
> In commit 8a5a75e5e9e5 ("of/fdt: Make sure no-map does not remove
> already reserved regions") we returned -EBUSY when trying to mark
> regions as no-map when they intersect with reserved memory. The goal was
> to find bad no-map reserved memory DT nodes that would unmap the kernel
> text/data sections.
> 
> The problem is the reserved memory check will still trigger if the DT
> has a /memreserve/ that completely subsumes the no-map memory carveouts
> in the reserved memory node _and_ that region is also not part of the
> memory reg property. For example in sc7180.dtsi we have the following
> reserved-memory and memory node:
> 
>       memory@...00000 {
>           /* We expect the bootloader to fill in the size */
>           reg = <0 0x80000000 0 0>;
>       };
> 
>       smem_mem: memory@...00000 {
>               reg = <0x0 0x80900000 0x0 0x200000>;
>               no-map;
>       };
> 
> and the memreserve filled in by the bootloader is
> 
>       /memreserve/ 0x80800000 0x400000;
> 
> while the /memory node is transformed into
> 
>       memory@...00000 {
>           /* The bootloader fills in the size, and adds another region */
>           reg = <0 0x80000000 0 0x00800000>,
>                 <0 0x80c00000 0 0x7f200000>;
>       };
> 
> The smem region is doubly reserved via /memreserve/ and by not being
> part of the /memory reg property. This leads to the following warning
> printed at boot.
> 
>  OF: fdt: Reserved memory: failed to reserve memory for node 'memory@...00000': base 0x0000000080900000, size 2 MiB
> 
> Otherwise nothing really goes wrong because the smem region is not going
> to be mapped by the kernel's direct linear mapping given that it isn't
> part of the memory node. Therefore, let's only consider this to be a
> problem if we're trying to mark a region as no-map and it is actually
> memory that we're intending to keep out of the kernel's direct mapping
> but it's already been reserved.
> 
> Acked-by: Mike Rapoport <rppt@...nel.org>
> Cc: Douglas Anderson <dianders@...omium.org>
> Cc: Nicolas Boichat <drinkcat@...omium.org>
> Cc: Quentin Perret <qperret@...gle.com>
> Cc: Jan Kiszka <jan.kiszka@...mens.com>
> Fixes: 8a5a75e5e9e5 ("of/fdt: Make sure no-map does not remove already reserved regions")
> Signed-off-by: Stephen Boyd <swboyd@...omium.org>
> ---
> 
> Changes from v3 (https://lore.kernel.org/r/20211215195354.634746-1-swboyd@chromium.org):
>  * Even more details in commit text
> 
> Changes from v2 (https://lore.kernel.org/r/20211215072011.496998-1-swboyd@chromium.org):
>  * More details in commit text
> 
> Changes from v1 (https://lore.kernel.org/r/20210520012731.3731314-1-swboyd@chromium.org):
>  * Use memblock_overlaps_region instead of memblock_is_region_memory()
>  * Add more details to commit text
> 
>  drivers/of/fdt.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 

Applied, thanks!