linux-kernel - Re: [PATCH] of: move empty_root and unittest data DTBs to .init.rodata section

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]

Message-ID: <20240827155158.GA3940418-robh@kernel.org>
Date: Tue, 27 Aug 2024 10:51:58 -0500
From: Rob Herring <robh@...nel.org>
To: Masahiro Yamada <masahiroy@...nel.org>
Cc: Saravana Kannan <saravanak@...gle.com>, devicetree@...r.kernel.org,
	linux-kernel@...r.kernel.org, Frank Rowand <frowand.list@...il.com>
Subject: Re: [PATCH] of: move empty_root and unittest data DTBs to
 .init.rodata section

On Mon, Aug 26, 2024 at 09:48:01PM +0900, Masahiro Yamada wrote:
> Some architectures can embed DTB(s) in vmlinux. Most of them expect a
> single DTB in the .dtb.init.rodata section.
> 
> For example, RISC-V previously allowed embedding multiple DTBs in
> vmlinux, but only the first DTB in the .dtb.init.rodata section was
> used. Which DTB was used was unpredictable, as it depended on the link
> order (i.e., the order in Makefile).
> 
> Commit 2672031b20f6 ("riscv: dts: Move BUILTIN_DTB_SOURCE to common
> Kconfig") changed the Makefiles to ensure only one DTB is embedded.
> 
> However, commit 7b937cc243e5 ("of: Create of_root if no dtb provided by
> firmware") introduced another DTB into the .dtb.init.rodata section.
> 
> Since then, the symbol dump (sorted by address) for ARCH=riscv
> nommu_k210_defconfig is as follows:
> 
>     00000000801290e0 D __dtb_k210_generic_begin
>     00000000801290e0 D __dtb_start
>     000000008012b571 D __dtb_k210_generic_end
>     000000008012b580 D __dtb_empty_root_begin
>     000000008012b5c8 D __dtb_empty_root_end
>     000000008012b5e0 D __dtb_end
> 
> The .dtb.init.rodata section now contains the following two DTB files:
> 
>     arch/riscv/boot/dts/canaan/k210_generic.dtb
>     drivers/of/empty_root.dtb
> 
> This is not an immediate problem because the boot code picks up the
> first DTB. The second one, empty_root.dtb is just ignored.
> 
> However, as mentioned above, it is fragile to rely on the link order,
> as future Makefile changes may break the behavior.
> 
> The cmd_wrap_S_dtb rule in scripts/Makefile.lib is used for embedding a
> DTB into the .dtb.init.rodata, so that the arch boot code can find it by
> the __dtb_start symbol.
> 
> empty_root.dtb is looked up by its own symbol, so it does not need to
> be located in the .dtb.init.rodata. It can be moved to the .init.rodata
> section.
> 
> When CONFIG_OF_UNITTEST is enabled, more unittest DTBOs are embedded in
> the .dtb.init.rodata section. These are also looked up by name and for
> generic purposes, so they can be moved to the .init.rodata section as
> well.
> 
> I added a wrapper source file, drivers/of/empty_root_dtb.S, because this
> is the only wrapper used in driver/of/Makefile. I moved the rule for
> generating *.dtbo.S to drivers/of/unittest-data/Makefile because it is
> not used anywhere else.

That is likely to change. We've had fixup overlays (fixup an old dt 
to a new binding) added into the kernel from time to time. There were 2, 
but they've been removed. However, I just recently suggested adding some 
new ones[1].

It seems we need a named section when we access the dtb by variable 
name and an unnamed or boot dt section for the one boot dtb.

Rob

[1] https://lore.kernel.org/all/20240812212139.GA1797954-robh@kernel.org/