linux-kernel - Re: [RFC PATCH 3/4] Kbuild: Create intermediate vmlinux build with relocations preserved

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: <CAK7LNATGEK+2=QXzE61YuzaMbZwS5JOVSFxhtCLWqfmt2QwFMA@mail.gmail.com>
Date: Sat, 8 Mar 2025 04:04:58 +0900
From: Masahiro Yamada <masahiroy@...nel.org>
To: Ard Biesheuvel <ardb+git@...gle.com>
Cc: linux-kernel@...r.kernel.org, x86@...nel.org, linux-kbuild@...r.kernel.org, 
	Ard Biesheuvel <ardb@...nel.org>, Ingo Molnar <mingo@...nel.org>
Subject: Re: [RFC PATCH 3/4] Kbuild: Create intermediate vmlinux build with
 relocations preserved

On Mon, Feb 24, 2025 at 10:22 PM Ard Biesheuvel <ardb+git@...gle.com> wrote:
>
> From: Ard Biesheuvel <ardb@...nel.org>
>
> The imperative paradigm used to build vmlinux, extract some info from it
> or perform some checks on it, and subsequently modify it again goes
> against the declarative paradigm that is usually employed for defining
> make rules.
>
> In particular, the Makefile.postlink files that consume their input via
> an output rule result in some dodgy logic in the decompressor makefiles
> for RISC-V and x86, given that the vmlinux.relocs input file needed to
> generate the arch-specific relocation tables may not exist or be out of
> date, but cannot be constructed using the ordinary Make dependency based
> rules, because the info needs to be extracted while vmlinux is in its
> ephemeral, non-stripped form.
>
> So instead, for architectures that require the static relocations that
> are emitted into vmlinux when passing --emit-relocs to the linker, and
> are subsequently stripped out again, introduce an intermediate vmlinux
> target called vmlinux.unstripped, and organize the reset of the build
> logic accordingly:
>
> - vmlinux.unstripped is created only once, and not updated again
> - build rules under arch/*/boot can depend on vmlinux.unstripped without
>   running the risk of the data disappearing or being out of date
> - the final vmlinux generated by the build is not bloated with static
>   relocations that are never needed again after the build completes.
>
> Signed-off-by: Ard Biesheuvel <ardb@...nel.org>
> ---

I already expressed my concern in the cover-letter.

Other than that, there are some nits:

 - You need to update .gitignore to ignore vmlinux.unstripped
  and the 'clean' target to clean up vmlinux.unstripped.

- cmd_strip_relocs can be moved to scripts/Makefile.vmlinux
  from scripts/Makefile.lib




>  arch/mips/Makefile.postlink  |  2 +-
>  arch/riscv/Makefile.postlink | 11 +--------
>  arch/riscv/boot/Makefile     |  5 +---
>  arch/s390/Makefile.postlink  |  4 +---
>  arch/x86/Makefile.postlink   |  8 +++----
>  scripts/Makefile.lib         |  2 +-
>  scripts/Makefile.vmlinux     | 25 ++++++++++++++------
>  7 files changed, 26 insertions(+), 31 deletions(-)
>
> diff --git a/arch/mips/Makefile.postlink b/arch/mips/Makefile.postlink
> index 6cfdc149d3bc..ea0add7d56b2 100644
> --- a/arch/mips/Makefile.postlink
> +++ b/arch/mips/Makefile.postlink
> @@ -22,7 +22,7 @@ quiet_cmd_relocs = RELOCS  $@
>
>  # `@...e` prevents complaint when there is nothing to be done
>
> -vmlinux: FORCE
> +vmlinux vmlinux.unstripped: FORCE
>         @true
>  ifeq ($(CONFIG_CPU_LOONGSON3_WORKAROUNDS),y)
>         $(call if_changed,ls3_llsc)
> diff --git a/arch/riscv/Makefile.postlink b/arch/riscv/Makefile.postlink
> index 6b0580949b6a..0e4cf8ad2f14 100644
> --- a/arch/riscv/Makefile.postlink
> +++ b/arch/riscv/Makefile.postlink
> @@ -10,26 +10,17 @@ __archpost:
>
>  -include include/config/auto.conf
>  include $(srctree)/scripts/Kbuild.include
> -include $(srctree)/scripts/Makefile.lib
>
>  quiet_cmd_relocs_check = CHKREL  $@
>  cmd_relocs_check =                                                     \
>         $(CONFIG_SHELL) $(srctree)/arch/riscv/tools/relocs_check.sh "$(OBJDUMP)" "$(NM)" "$@"
>
> -ifdef CONFIG_RELOCATABLE
> -quiet_cmd_cp_vmlinux_relocs = CPREL   vmlinux.relocs
> -cmd_cp_vmlinux_relocs = cp vmlinux vmlinux.relocs
> -
> -endif
> -
>  # `@...e` prevents complaint when there is nothing to be done
>
> -vmlinux: FORCE
> +vmlinux vmlinux.unstripped: FORCE
>         @true
>  ifdef CONFIG_RELOCATABLE
>         $(call if_changed,relocs_check)
> -       $(call if_changed,cp_vmlinux_relocs)
> -       $(call if_changed,strip_relocs)
>  endif
>
>  clean:
> diff --git a/arch/riscv/boot/Makefile b/arch/riscv/boot/Makefile
> index b25d524ce5eb..bfc3d0b75b9b 100644
> --- a/arch/riscv/boot/Makefile
> +++ b/arch/riscv/boot/Makefile
> @@ -32,10 +32,7 @@ $(obj)/xipImage: vmlinux FORCE
>  endif
>
>  ifdef CONFIG_RELOCATABLE
> -vmlinux.relocs: vmlinux
> -       @ (! [ -f vmlinux.relocs ] && echo "vmlinux.relocs can't be found, please remove vmlinux and try again") || true
> -
> -$(obj)/Image: vmlinux.relocs FORCE
> +$(obj)/Image: vmlinux.unstripped FORCE
>  else
>  $(obj)/Image: vmlinux FORCE
>  endif
> diff --git a/arch/s390/Makefile.postlink b/arch/s390/Makefile.postlink
> index 1ae5478cd6ac..c2b737500a91 100644
> --- a/arch/s390/Makefile.postlink
> +++ b/arch/s390/Makefile.postlink
> @@ -11,7 +11,6 @@ __archpost:
>
>  -include include/config/auto.conf
>  include $(srctree)/scripts/Kbuild.include
> -include $(srctree)/scripts/Makefile.lib
>
>  CMD_RELOCS=arch/s390/tools/relocs
>  OUT_RELOCS = arch/s390/boot
> @@ -20,9 +19,8 @@ quiet_cmd_relocs = RELOCS  $(OUT_RELOCS)/relocs.S
>         mkdir -p $(OUT_RELOCS); \
>         $(CMD_RELOCS) $@ > $(OUT_RELOCS)/relocs.S
>
> -vmlinux: FORCE
> +vmlinux.unstripped: FORCE
>         $(call cmd,relocs)
> -       $(call cmd,strip_relocs)
>
>  clean:
>         @rm -f $(OUT_RELOCS)/relocs.S
> diff --git a/arch/x86/Makefile.postlink b/arch/x86/Makefile.postlink
> index 8b8a68162c94..445fce66630f 100644
> --- a/arch/x86/Makefile.postlink
> +++ b/arch/x86/Makefile.postlink
> @@ -11,23 +11,21 @@ __archpost:
>
>  -include include/config/auto.conf
>  include $(srctree)/scripts/Kbuild.include
> -include $(srctree)/scripts/Makefile.lib
>
>  CMD_RELOCS = arch/x86/tools/relocs
>  OUT_RELOCS = arch/x86/boot/compressed
> -quiet_cmd_relocs = RELOCS  $(OUT_RELOCS)/$@...locs
> +quiet_cmd_relocs = RELOCS  $(OUT_RELOCS)/vmlinux.relocs
>        cmd_relocs = \
>         mkdir -p $(OUT_RELOCS); \
> -       $(CMD_RELOCS) $@ > $(OUT_RELOCS)/$@...locs; \
> +       $(CMD_RELOCS) $@ > $(OUT_RELOCS)/vmlinux.relocs; \
>         $(CMD_RELOCS) --abs-relocs $@
>
>  # `@...e` prevents complaint when there is nothing to be done
>
> -vmlinux: FORCE
> +vmlinux vmlinux.unstripped: FORCE
>         @true
>  ifeq ($(CONFIG_X86_NEED_RELOCS),y)
>         $(call cmd,relocs)
> -       $(call cmd,strip_relocs)
>  endif
>
>  clean:
> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
> index cad20f0e66ee..7a023f17a21d 100644
> --- a/scripts/Makefile.lib
> +++ b/scripts/Makefile.lib
> @@ -377,7 +377,7 @@ quiet_cmd_objcopy = OBJCOPY $@
>  cmd_objcopy = $(OBJCOPY) $(OBJCOPYFLAGS) $(OBJCOPYFLAGS_$(@F)) $< $@
>
>  quiet_cmd_strip_relocs = RSTRIP  $@
> -cmd_strip_relocs = $(OBJCOPY) --remove-section='.rel*' $@
> +cmd_strip_relocs = $(OBJCOPY) --remove-section='.rel*' $< $@
>
>  # Gzip
>  # ---------------------------------------------------------------------------
> diff --git a/scripts/Makefile.vmlinux b/scripts/Makefile.vmlinux
> index 3523ce3ce3dc..1e3ea8e4b4b0 100644
> --- a/scripts/Makefile.vmlinux
> +++ b/scripts/Makefile.vmlinux
> @@ -9,6 +9,17 @@ include $(srctree)/scripts/Makefile.lib
>
>  targets :=
>
> +ifdef CONFIG_ARCH_VMLINUX_NEEDS_RELOCS
> +vmlinux-final := vmlinux.unstripped
> +
> +vmlinux: $(vmlinux-final) FORCE
> +       $(call if_changed,strip_relocs)
> +
> +targets += vmlinux
> +else
> +vmlinux-final := vmlinux
> +endif
> +
>  %.o: %.c FORCE
>         $(call if_changed_rule,cc_o_c)
>
> @@ -47,7 +58,7 @@ targets += .builtin-dtbs-list
>
>  ifdef CONFIG_GENERIC_BUILTIN_DTB
>  targets += .builtin-dtbs.S .builtin-dtbs.o
> -vmlinux: .builtin-dtbs.o
> +$(vmlinux-final): .builtin-dtbs.o
>  endif
>
>  # vmlinux
> @@ -55,11 +66,11 @@ endif
>
>  ifdef CONFIG_MODULES
>  targets += .vmlinux.export.o
> -vmlinux: .vmlinux.export.o
> +$(vmlinux-final): .vmlinux.export.o
>  endif
>
>  ifdef CONFIG_ARCH_WANTS_PRE_LINK_VMLINUX
> -vmlinux: arch/$(SRCARCH)/tools/vmlinux.arch.o
> +$(vmlinux-final): arch/$(SRCARCH)/tools/vmlinux.arch.o
>
>  arch/$(SRCARCH)/tools/vmlinux.arch.o: vmlinux.o FORCE
>         $(Q)$(MAKE) $(build)=arch/$(SRCARCH)/tools $@
> @@ -72,11 +83,11 @@ cmd_link_vmlinux =                                                  \
>         $< "$(LD)" "$(KBUILD_LDFLAGS)" "$(LDFLAGS_vmlinux)" "$@";       \
>         $(if $(ARCH_POSTLINK), $(MAKE) -f $(ARCH_POSTLINK) $@, true)
>
> -targets += vmlinux
> -vmlinux: scripts/link-vmlinux.sh vmlinux.o $(KBUILD_LDS) FORCE
> +targets += $(vmlinux-final)
> +$(vmlinux-final): scripts/link-vmlinux.sh vmlinux.o $(KBUILD_LDS) FORCE
>         +$(call if_changed_dep,link_vmlinux)
>  ifdef CONFIG_DEBUG_INFO_BTF
> -vmlinux: $(RESOLVE_BTFIDS)
> +$(vmlinux-final): $(RESOLVE_BTFIDS)
>  endif
>
>  # module.builtin.ranges
> @@ -92,7 +103,7 @@ modules.builtin.ranges: $(srctree)/scripts/generate_builtin_ranges.awk \
>                         modules.builtin vmlinux.map vmlinux.o.map FORCE
>         $(call if_changed,modules_builtin_ranges)
>
> -vmlinux.map: vmlinux
> +vmlinux.map: $(vmlinux-final)
>         @:
>
>  endif
> --
> 2.48.1.601.g30ceb7b040-goog
>
>


-- 
Best Regards
Masahiro Yamada