linux-kernel - Re: [PATCH v3 1/2] kbuild: move non-boot built-in DTBs to .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] [day] [month] [year] [list]

Message-ID: <CAK7LNARrs18T74-+JOnLCPqP1-nJEnVaOAGOZVDOvxEH0y2_pg@mail.gmail.com>
Date: Fri, 27 Sep 2024 23:51:57 +0900
From: Masahiro Yamada <masahiroy@...nel.org>
To: linux-kbuild@...r.kernel.org
Cc: linux-kernel@...r.kernel.org, Andrea della Porta <andrea.porta@...e.com>, 
	linux-clk@...r.kernel.org, Stephen Boyd <sboyd@...nel.org>, 
	Brendan Higgins <brendanhiggins@...gle.com>, Rob Herring <robh@...nel.org>, devicetree@...r.kernel.org, 
	Nathan Chancellor <nathan@...nel.org>, Nicolas Schier <nicolas@...sle.eu>
Subject: Re: [PATCH v3 1/2] kbuild: move non-boot built-in DTBs to .rodata section

On Mon, Sep 23, 2024 at 4:57 PM Masahiro Yamada <masahiroy@...nel.org> wrote:
>
> Commit aab94339cd85 ("of: Add support for linking device tree blobs
> into vmlinux") introduced a mechanism to embed DTBs into vmlinux.
>
> Initially, it was used for wrapping boot DTBs in arch/*/boot/dts/, but
> it is now reused for more generic purposes, such as testing.
>
> Built-in DTBs are discarded because KERNEL_DTB() is part of INIT_DATA,
> as defined in include/asm-generic/vmlinux.lds.h.
>
> This has not been an issue so far because OF unittests are triggered
> during boot, as defined by late_initcall(of_unittest).
>
> However, the recent clk KUnit test additions have caused problems
> because KUnit can execute test suites after boot.
>
> For example:
>
>   # echo > /sys/kernel/debug/kunit/clk_register_clk_parent_data_device/run
>
> This command triggers a stack trace because built-in DTBs have already
> been freed.
>
> While it is possible to move such test suites from kunit_test_suites to
> kunit_test_init_section_suites, it would be preferable to avoid usage
> limitations.
>
> This commit moves non-boot built-in DTBs to the .rodata section. Since
> these generic DTBs are looked up by name, they do not need to be placed
> in the special .dtb.init.rodata section.
>
> Boot DTBs should remain in .dtb.init.rodata because the arch boot code
> generally does not know the DT name, thus it uses the __dtb_start symbol
> to locate it.
>
> This separation also ensures that the __dtb_start symbol references the
> boot DTB. Currently, the .dtb.init.rodata is a mixture of both boot and
> non-boot DTBs. The __dtb_start symbol must be followed by the boot DTB,
> but we currently rely on the link order (i.e., the order in Makefiles),
> which is very fragile.
>
> Fixes: 5c9dd72d8385 ("of: Add a KUnit test for overlays and test managed APIs")
> Fixes: 5776526beb95 ("clk: Add KUnit tests for clk fixed rate basic type")
> Fixes: 274aff8711b2 ("clk: Add KUnit tests for clks registered with struct clk_parent_data")
> Signed-off-by: Masahiro Yamada <masahiroy@...nel.org>
> ---
>
> Changes in v3:
>   - Move to .rodata section instead of .init.rodata
>
>  scripts/Makefile.dtbs | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/scripts/Makefile.dtbs b/scripts/Makefile.dtbs
> index 46009d5f1486..3e81cca6f68a 100644
> --- a/scripts/Makefile.dtbs
> +++ b/scripts/Makefile.dtbs
> @@ -34,12 +34,14 @@ $(obj)/dtbs-list: $(dtb-y) FORCE
>  # Assembly file to wrap dtb(o)
>  # ---------------------------------------------------------------------------
>
> +builtin-dtb-section = $(if $(filter arch/%, $(obj)),.dtb.init.rodata,.rodata)


For more precise matching, I will change as follows:


diff --git a/scripts/Makefile.dtbs b/scripts/Makefile.dtbs
index 3e81cca6f68a..8d56c0815f33 100644
--- a/scripts/Makefile.dtbs
+++ b/scripts/Makefile.dtbs
@@ -34,7 +34,7 @@ $(obj)/dtbs-list: $(dtb-y) FORCE
 # Assembly file to wrap dtb(o)
 # ---------------------------------------------------------------------------

-builtin-dtb-section = $(if $(filter arch/%, $(obj)),.dtb.init.rodata,.rodata)
+builtin-dtb-section = $(if $(filter arch/$(SRCARCH)/boot/dts%,
$(obj)),.dtb.init.rodata,.rodata)

 # Generate an assembly file to wrap the output of the device tree compiler
 quiet_cmd_wrap_S_dtb = WRAP    $@



Applied to linux-kbuild/fixes.




> +
>  # Generate an assembly file to wrap the output of the device tree compiler
>  quiet_cmd_wrap_S_dtb = WRAP    $@
>        cmd_wrap_S_dtb = {                                                               \
>                 symbase=__$(patsubst .%,%,$(suffix $<))_$(subst -,_,$(notdir $*));      \
>                 echo '\#include <asm-generic/vmlinux.lds.h>';                           \
> -               echo '.section .dtb.init.rodata,"a"';                                   \
> +               echo '.section $(builtin-dtb-section),"a"';                             \
>                 echo '.balign STRUCT_ALIGNMENT';                                        \
>                 echo ".global $${symbase}_begin";                                       \
>                 echo "$${symbase}_begin:";                                              \
> --
> 2.43.0
>


-- 
Best Regards
Masahiro Yamada