[RFC PATCH 00/11] output a valid shell script when running 'make -n'

[RFC PATCH 00/11] output a valid shell script when running 'make -n'

[Vegard Nossum]

This patch series lets 'make -n' output a shell script that can be
used to build the kernel without any further use of make. For example:

    make defconfig

    # ensure some build prerequisites are built
    make prepare

    # generate build script
    make -n | tee build.sh

    # excecute build script
    bash -eux build.sh

The purpose of this is to take a step towards defeating the insertion of
backdoors at build time (see [1]). Some of the benefits of separating the
build script from the build system are:

 - we can invoke make in a restricted environment (e.g. mostly read-only
   kernel tree),

 - we have an audit log of the exact commands that run during the build
   process; although it's true that the build script wouldn't be useful
   for either production or development builds (as it doesn't support
   incremental rebuilds or parallel builds), it would allow you to
   rebuild an existing kernel and compare the resulting binary for
   discrepancies to the original build,

 - the audit log can be stored (e.g. in git) and changes to it over time
   can themselves be audited (e.g. by looking at diffs),

 - there's a lot fewer places to hide malicious code in a straight-line
   shell script that makes minimal use of variables and helper functions.
   You also cannot inject fragments of Makefile code through environment
   variables (again, see [1]).

Alternative ways to achieve some of the same things would be:

 - the existing compile_commands.json infrastructure; unfortunately this
   does not include most of the steps performed during a build (such as
   linking vmlinux) and does not really allow you to reproduce/verify the
   full build,

 - simply running something like strace -f -e trace=execve make; however,
   this also does not result in something that can be easily played back;
   at the very least it would need to be heavily filtered and processed
   to account for data passed in environment variables and things like
   temporary files used by the compiler.

This implementation works as follows:

 - 'make -n' (AKA --dry-run) by default prints out the commands that make
   runs; this output is modified to be usable as a shell script,

 - we output 'make() { :; }' at the start of the script in order to make
   all 'make' invocations in the resulting build script no-ops (GNU Make
   will actually execute -- and print -- all recipe lines that include
   $(MAKE), even when invoked with -n).

 - we simplify the makefile rules in some cases to make the shell script
   more readable; for example, we don't need the logic that extracts
   dependencies from .c files (since that is only used by 'make' itself
   when determining what to rebuild) or the logic that generates .cmd
   files,

This patch is WIP and may not produce a working shell script in all
circumstances. For example, while plain 'make -n' works for me, other
make targets (e.g. 'make -n htmldocs') are not at all guaranteed to
produce meaningful output; certain kernel configs may also not work,
especially those that rely on external tools like e.g. Rust.

[1]: https://www.openwall.com/lists/oss-security/2024/04/17/3
[2]: https://www.gnu.org/software/make/manual/make.html#Testing-Flags


Vegard

---

Vegard Nossum (11):
  kbuild: ignore .config rule for make --always-make
  kbuild: document some prerequisites
  kbuild: pass KERNELVERSION and LOCALVERSION explicitly to
    setlocalversion
  kbuild: don't execute .ko recipe in --dry-run mode
  kbuild: execute modules.order recipe in --dry-run mode
  kbuild: set $dry_run when running in --dry-run mode
  kbuild: define 'make' as a no-op in --dry-run mode
  kbuild: make link-vmlinux.sh respect $dry_run
  kbuild: simplify commands in --dry-run mode
  kbuild: don't test for file presence in --dry-run mode
  kbuild: suppress echoing of commands in --dry-run mode

 Makefile                          | 28 +++++++++++++++++---
 arch/x86/boot/compressed/Makefile |  6 +++++
 scripts/Kbuild.include            | 27 +++++++++++++++++++
 scripts/Makefile.build            |  2 +-
 scripts/Makefile.modfinal         |  9 +++++--
 scripts/Makefile.modpost          |  8 ++++--
 scripts/Makefile.vmlinux          | 22 ++++++++++++++--
 scripts/Makefile.vmlinux_o        |  3 +++
 scripts/link-vmlinux.sh           | 44 ++++++++++++++++++++-----------
 9 files changed, 123 insertions(+), 26 deletions(-)

-- 
2.34.1

[RFC PATCH 01/11] kbuild: ignore .config rule for make --always-make

[Vegard Nossum]

Before this patch, using 'make --always-make' would always result in the
error message about the missing .config being displayed.

Detect the -B/--always-make flag and leave this rule out, which allows the
rest of the build to proceed. See [1] for an explanation of this particular
construction.

[1]: https://www.gnu.org/software/make/manual/make.html#Testing-Flags

Signed-off-by: Vegard Nossum <vegard.nossum@oracle.com>
---
 Makefile | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Makefile b/Makefile
index 44c02a6f60a14..f09c036daf2f5 100644
--- a/Makefile
+++ b/Makefile
@@ -757,6 +757,10 @@ ifdef may-sync-config
 # because some architectures define CROSS_COMPILE there.
 include include/config/auto.conf.cmd
 
+ifeq (,$(findstring B,$(firstword -$(MAKEFLAGS))))
+# This is a dummy target, only meant as a help for the user invoking make.
+# We don't want it to take effect when running 'make --always-make', since
+# that renders the --always-make option effectively useless.
 $(KCONFIG_CONFIG):
 	@echo >&2 '***'
 	@echo >&2 '*** Configuration file "$@" not found!'
@@ -765,6 +769,7 @@ $(KCONFIG_CONFIG):
 	@echo >&2 '*** "make menuconfig" or "make xconfig").'
 	@echo >&2 '***'
 	@/bin/false
+endif
 
 # The actual configuration files used during the build are stored in
 # include/generated/ and include/config/. Update them if .config is newer than
-- 
2.34.1

[RFC PATCH 02/11] kbuild: document some prerequisites

[Vegard Nossum]

When running 'make --dry-run', make will invoke itself recursively for
recipes using $(MAKE), but otherwise not execute the other commands in
a recipe.

However, if a prerequisite is missing and 'make' does not how to create it
(which will be the case when running 'make -n'), it will complain with an
error message like this:

  $ make -n
  ...
  make -f ./scripts/Makefile.modpost
  make[1]: *** No rule to make target 'modules.order', needed by 'modules-only.symvers'.  Stop.
  make: *** [Makefile:1868: modpost] Error 2

In this case, the top-level makefile has reached a recipe that ran 'make'
recursively on scripts/Makefile.modpost, which itself has a rule with
modules.order as a prerequisite. Since the file doesn't exist, and make
doesn't know how to create it, it errors out.

We can document such prerequisites (which are expected to be created by
the parent Makefile) by adding them to the PHONY list of each respective
Makefile.

Signed-off-by: Vegard Nossum <vegard.nossum@oracle.com>
---
 arch/x86/boot/compressed/Makefile | 6 ++++++
 scripts/Makefile.modfinal         | 5 +++++
 scripts/Makefile.modpost          | 4 ++++
 scripts/Makefile.vmlinux          | 7 +++++++
 scripts/Makefile.vmlinux_o        | 3 +++
 5 files changed, 25 insertions(+)

diff --git a/arch/x86/boot/compressed/Makefile b/arch/x86/boot/compressed/Makefile
index f2051644de943..ccef6f0e04bc7 100644
--- a/arch/x86/boot/compressed/Makefile
+++ b/arch/x86/boot/compressed/Makefile
@@ -80,6 +80,9 @@ quiet_cmd_voffset = VOFFSET $@
 
 targets += ../voffset.h
 
+# We don't know how to build this
+PHONY += vmlinux
+
 $(obj)/../voffset.h: vmlinux FORCE
 	$(call if_changed,voffset)
 
@@ -107,6 +110,9 @@ vmlinux-objs-$(CONFIG_EFI) += $(obj)/efi.o
 vmlinux-objs-$(CONFIG_EFI_MIXED) += $(obj)/efi_mixed.o
 vmlinux-libs-$(CONFIG_EFI_STUB) += $(objtree)/drivers/firmware/efi/libstub/lib.a
 
+# We don't know how to build this
+PHONY += $(objtree)/drivers/firmware/efi/libstub/lib.a
+
 $(obj)/vmlinux: $(vmlinux-objs-y) $(vmlinux-libs-y) FORCE
 	$(call if_changed,ld)
 
diff --git a/scripts/Makefile.modfinal b/scripts/Makefile.modfinal
index 1fa98b5e952b4..696888f0a0bde 100644
--- a/scripts/Makefile.modfinal
+++ b/scripts/Makefile.modfinal
@@ -15,6 +15,11 @@ include $(srctree)/scripts/Makefile.lib
 # find all modules listed in modules.order
 modules := $(call read-file, $(MODORDER))
 
+# We don't know how to build these
+PHONY += $(modules)
+PHONY += $(modules:%.o=%.mod.c)
+PHONY += scripts/module.lds
+
 __modfinal: $(modules:%.o=%.ko)
 	@:
 
diff --git a/scripts/Makefile.modpost b/scripts/Makefile.modpost
index 44936ebad161e..65f2bdc702369 100644
--- a/scripts/Makefile.modpost
+++ b/scripts/Makefile.modpost
@@ -140,6 +140,10 @@ quiet_cmd_modpost = MODPOST $@
 		echo >&2 "         if you want to proceed at your own risk.";) \
 	$(MODPOST) $(modpost-args)
 
+# We don't know how to build these
+PHONY += vmlinux.o
+PHONY += $(MODORDER)
+
 targets += $(output-symdump)
 $(output-symdump): $(modpost-deps) FORCE
 	$(call if_changed,modpost)
diff --git a/scripts/Makefile.vmlinux b/scripts/Makefile.vmlinux
index 49946cb968440..10d80e07f945c 100644
--- a/scripts/Makefile.vmlinux
+++ b/scripts/Makefile.vmlinux
@@ -18,6 +18,9 @@ quiet_cmd_cc_o_c = CC      $@
 	$(call if_changed_dep,cc_o_c)
 
 ifdef CONFIG_MODULES
+# We don't know how to build this
+PHONY += .vmlinux.export.c
+
 targets += .vmlinux.export.o
 vmlinux: .vmlinux.export.o
 endif
@@ -29,6 +32,10 @@ cmd_link_vmlinux =							\
 	$< "$(LD)" "$(KBUILD_LDFLAGS)" "$(LDFLAGS_vmlinux)";		\
 	$(if $(ARCH_POSTLINK), $(MAKE) -f $(ARCH_POSTLINK) $@, true)
 
+# We don't know how to build these
+PHONY += vmlinux.o
+PHONY += $(KBUILD_LDS)
+
 targets += vmlinux
 vmlinux: scripts/link-vmlinux.sh vmlinux.o $(KBUILD_LDS) FORCE
 	+$(call if_changed_dep,link_vmlinux)
diff --git a/scripts/Makefile.vmlinux_o b/scripts/Makefile.vmlinux_o
index 6de297916ce68..7af841eb1ce5b 100644
--- a/scripts/Makefile.vmlinux_o
+++ b/scripts/Makefile.vmlinux_o
@@ -58,6 +58,9 @@ define rule_ld_vmlinux.o
 	$(call cmd,gen_objtooldep)
 endef
 
+# We don't know how to build this
+PHONY += vmlinux.a
+
 vmlinux.o: $(initcalls-lds) vmlinux.a $(KBUILD_VMLINUX_LIBS) FORCE
 	$(call if_changed_rule,ld_vmlinux.o)
 
-- 
2.34.1

[RFC PATCH 03/11] kbuild: pass KERNELVERSION and LOCALVERSION explicitly to setlocalversion

[Vegard Nossum]

These environment variables are passed when invoking 'make', but if
running 'make -n' we need to pass them explicitly so they become part
of the printed command.

Signed-off-by: Vegard Nossum <vegard.nossum@oracle.com>
---
 Makefile | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index f09c036daf2f5..58f3843ccfac6 100644
--- a/Makefile
+++ b/Makefile
@@ -1165,7 +1165,10 @@ vmlinux: vmlinux.o $(KBUILD_LDS) modpost
 $(sort $(KBUILD_LDS) $(KBUILD_VMLINUX_OBJS) $(KBUILD_VMLINUX_LIBS)): . ;
 
 ifeq ($(origin KERNELRELEASE),file)
-filechk_kernel.release = $(srctree)/scripts/setlocalversion $(srctree)
+filechk_kernel.release = \
+	KERNELVERSION="$(KERNELVERSION)" \
+	LOCALVERSION="$(LOCALVERSION)" \
+	$(srctree)/scripts/setlocalversion $(srctree)
 else
 filechk_kernel.release = echo $(KERNELRELEASE)
 endif
-- 
2.34.1

[RFC PATCH 04/11] kbuild: don't execute .ko recipe in --dry-run mode

[Vegard Nossum]

Prefixing a line in a make recipe with + makes that command execute even
in --dry-run mode. We don't need that here; remove it.

Fixes: 5f9ae91f7c0d ("kbuild: Build kernel module BTFs if BTF is enabled and pahole supports it")
Cc: Andrii Nakryiko <andrii@kernel.org>
Cc: Alexei Starovoitov <ast@kernel.org>
Signed-off-by: Vegard Nossum <vegard.nossum@oracle.com>
---
 scripts/Makefile.modfinal | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/scripts/Makefile.modfinal b/scripts/Makefile.modfinal
index 696888f0a0bde..2679304f158ad 100644
--- a/scripts/Makefile.modfinal
+++ b/scripts/Makefile.modfinal
@@ -60,9 +60,9 @@ if_changed_except = $(if $(call newer_prereqs_except,$(2))$(cmd-check),      \
 
 # Re-generate module BTFs if either module's .ko or vmlinux changed
 %.ko: %.o %.mod.o scripts/module.lds $(and $(CONFIG_DEBUG_INFO_BTF_MODULES),$(KBUILD_BUILTIN),vmlinux) FORCE
-	+$(call if_changed_except,ld_ko_o,vmlinux)
+	$(call if_changed_except,ld_ko_o,vmlinux)
 ifdef CONFIG_DEBUG_INFO_BTF_MODULES
-	+$(if $(newer-prereqs),$(call cmd,btf_ko))
+	$(if $(newer-prereqs),$(call cmd,btf_ko))
 endif
 
 targets += $(modules:%.o=%.ko) $(modules:%.o=%.mod.o)
-- 
2.34.1

[RFC PATCH 05/11] kbuild: execute modules.order recipe in --dry-run mode

[Vegard Nossum]

modules.order is read by scripts/Makefile.modfinal to determine which
modules to build, so we need this recipe to execute if we want to be able
to output the recipes for building modules in dry-run mode.

Signed-off-by: Vegard Nossum <vegard.nossum@oracle.com>
---
 scripts/Makefile.build | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index efacca63c8976..34f86dced67f3 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -417,7 +417,7 @@ cmd_gen_order = { $(foreach m, $(real-prereqs), \
 	> $@
 
 $(obj)/modules.order: $(obj-m) FORCE
-	$(call if_changed,gen_order)
+	+$(call if_changed,gen_order)
 
 $(obj)/dtbs-list: $(dtb-y) FORCE
 	$(call if_changed,gen_order)
-- 
2.34.1

[RFC PATCH 06/11] kbuild: set $dry_run when running in --dry-run mode

[Vegard Nossum]

Add a convenience variable that allows us to use 'ifdef dry_run...endif'
in Makefiles or '[ -v dry_run ]' in shell scripts to test whether make
was invoked with '-n'.

See [1] for an explanation of this particular construction.

[1]: https://www.gnu.org/software/make/manual/make.html#Testing-Flags

Signed-off-by: Vegard Nossum <vegard.nossum@oracle.com>
---
 Makefile | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Makefile b/Makefile
index 58f3843ccfac6..953951157ec92 100644
--- a/Makefile
+++ b/Makefile
@@ -22,6 +22,11 @@ $(if $(filter __%, $(MAKECMDGOALS)), \
 PHONY := __all
 __all:
 
+# Was make invoked with --dry-run/-n? Record this in a convenience variable.
+ifeq (n,$(findstring n,$(firstword -$(MAKEFLAGS))))
+export dry_run := 1
+endif
+
 # We are using a recursive build, so we need to do a little thinking
 # to get the ordering right.
 #
-- 
2.34.1

[RFC PATCH 07/11] kbuild: define 'make' as a no-op in --dry-run mode

[Vegard Nossum]

Output 'make() { :; }' at the start of the script in order to make
all 'make' invocations in the resulting build script no-ops (GNU Make
will actually execute -- and print -- all recipe lines that include
$(MAKE), even when invoked with -n).

Signed-off-by: Vegard Nossum <vegard.nossum@oracle.com>
---
 Makefile | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Makefile b/Makefile
index 953951157ec92..d08ade5791c2e 100644
--- a/Makefile
+++ b/Makefile
@@ -226,6 +226,12 @@ $(filter-out $(this-makefile), $(MAKECMDGOALS)) __all: __sub-make
 
 # Invoke a second make in the output directory, passing relevant variables
 __sub-make:
+ifdef dry_run
+	# define 'make' as a no-op alias so that those commands are not
+	# actually run
+	@make() { :; }
+	@export -f make
+endif
 	$(Q)$(MAKE) $(no-print-directory) -C $(abs_objtree) \
 	-f $(abs_srctree)/Makefile $(MAKECMDGOALS)
 
-- 
2.34.1

[RFC PATCH 08/11] kbuild: make link-vmlinux.sh respect $dry_run

[Vegard Nossum]

Make link-vmlinux.sh print the commands it wants to run instead of
actually running them when $dry_run is defined.

This is needed in order for make -n/--dry-run to output a complete
build script.

Signed-off-by: Vegard Nossum <vegard.nossum@oracle.com>
---
 scripts/Makefile.vmlinux | 15 ++++++++++++--
 scripts/link-vmlinux.sh  | 44 ++++++++++++++++++++++++++--------------
 2 files changed, 42 insertions(+), 17 deletions(-)

diff --git a/scripts/Makefile.vmlinux b/scripts/Makefile.vmlinux
index 10d80e07f945c..1fe0faab52889 100644
--- a/scripts/Makefile.vmlinux
+++ b/scripts/Makefile.vmlinux
@@ -1,5 +1,12 @@
 # SPDX-License-Identifier: GPL-2.0-only
 
+# there doesn't seem to be a straightforward way for 'make -n' to output
+# one command while actually executing another, so we resort to this hack
+# where we set an environment variable differently depending on whether
+# we are executing from 'make' or executing from the 'make -n' log.
+LINK_VMLINUX=scripts/link-vmlinux.sh
+export LINK_VMLINUX
+
 PHONY := __default
 __default: vmlinux
 
@@ -29,7 +36,8 @@ ARCH_POSTLINK := $(wildcard $(srctree)/arch/$(SRCARCH)/Makefile.postlink)
 
 # Final link of vmlinux with optional arch pass after final link
 cmd_link_vmlinux =							\
-	$< "$(LD)" "$(KBUILD_LDFLAGS)" "$(LDFLAGS_vmlinux)";		\
+	"$${LINK_VMLINUX}"						\
+	"$(LD)" "$(KBUILD_LDFLAGS)" "$(LDFLAGS_vmlinux)";		\
 	$(if $(ARCH_POSTLINK), $(MAKE) -f $(ARCH_POSTLINK) $@, true)
 
 # We don't know how to build these
@@ -37,7 +45,10 @@ PHONY += vmlinux.o
 PHONY += $(KBUILD_LDS)
 
 targets += vmlinux
-vmlinux: scripts/link-vmlinux.sh vmlinux.o $(KBUILD_LDS) FORCE
+vmlinux: $(LINK_VMLINUX) vmlinux.o $(KBUILD_LDS) FORCE
+ifdef dry_run
+	@LINK_VMLINUX=/bin/true
+endif
 	+$(call if_changed_dep,link_vmlinux)
 
 # Add FORCE to the prequisites of a target to force it to be always rebuilt.
diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
index f7b2503cdba95..fd32e48a8a455 100755
--- a/scripts/link-vmlinux.sh
+++ b/scripts/link-vmlinux.sh
@@ -32,6 +32,17 @@ LD="$1"
 KBUILD_LDFLAGS="$2"
 LDFLAGS_vmlinux="$3"
 
+# If $dry_run is set, just echo the command instead of executing it.
+run()
+{
+	if [ ! -v dry_run ]
+	then
+		eval "$1"
+	else
+		echo "$1"
+	fi
+}
+
 is_enabled() {
 	grep -q "^$1=y" include/config/auto.conf
 }
@@ -40,7 +51,10 @@ is_enabled() {
 # Will be supressed by "make -s"
 info()
 {
-	printf "  %-7s %s\n" "${1}" "${2}"
+	if [ ! -v dry_run ]
+	then
+		printf "  %-7s %s\n" "${1}" "${2}"
+	fi
 }
 
 # Link of vmlinux
@@ -97,10 +111,10 @@ vmlinux_link()
 		ldflags="${ldflags} ${wl}-Map=${output}.map"
 	fi
 
-	${ld} ${ldflags} -o ${output}					\
+	run "${ld} ${ldflags} -o ${output}				\
 		${wl}--whole-archive ${objs} ${wl}--no-whole-archive	\
 		${wl}--start-group ${libs} ${wl}--end-group		\
-		${kallsymso} ${btf_vmlinux_bin_o} ${ldlibs}
+		${kallsymso} ${btf_vmlinux_bin_o} ${ldlibs}"
 }
 
 # generate .BTF typeinfo from DWARF debuginfo
@@ -129,8 +143,8 @@ gen_btf()
 	# deletes all symbols including __start_BTF and __stop_BTF, which will
 	# be redefined in the linker script. Add 2>/dev/null to suppress GNU
 	# objcopy warnings: "empty loadable segment detected at ..."
-	${OBJCOPY} --only-section=.BTF --set-section-flags .BTF=alloc,readonly \
-		--strip-all ${1} "${btf_data}" 2>/dev/null
+	run "${OBJCOPY} --only-section=.BTF --set-section-flags .BTF=alloc,readonly \
+		--strip-all ${1} "${btf_data}" 2>/dev/null"
 	# Change e_type to ET_REL so that it can be used to link final vmlinux.
 	# GNU ld 2.35+ and lld do not allow an ET_EXEC input.
 	if is_enabled CONFIG_CPU_BIG_ENDIAN; then
@@ -161,11 +175,11 @@ kallsyms()
 	fi
 
 	info KSYMS "${2}.S"
-	scripts/kallsyms ${kallsymopt} "${1}" > "${2}.S"
+	run "scripts/kallsyms ${kallsymopt} \"${1}\" > \"${2}.S\""
 
 	info AS "${2}.o"
-	${CC} ${NOSTDINC_FLAGS} ${LINUXINCLUDE} ${KBUILD_CPPFLAGS} \
-	      ${KBUILD_AFLAGS} ${KBUILD_AFLAGS_KERNEL} -c -o "${2}.o" "${2}.S"
+	run "${CC} ${NOSTDINC_FLAGS} ${LINUXINCLUDE} ${KBUILD_CPPFLAGS} \
+	      ${KBUILD_AFLAGS} ${KBUILD_AFLAGS_KERNEL} -c -o \"${2}.o\" \"${2}.S\""
 
 	kallsymso=${2}.o
 }
@@ -184,12 +198,12 @@ sysmap_and_kallsyms()
 mksysmap()
 {
 	info NM ${2}
-	${NM} -n "${1}" | sed -f "${srctree}/scripts/mksysmap" > "${2}"
+	run "${NM} -n \"${1}\" | sed -f \"${srctree}/scripts/mksysmap\" > \"${2}\""
 }
 
 sorttable()
 {
-	${objtree}/scripts/sorttable ${1}
+	run "${objtree}/scripts/sorttable ${1}"
 }
 
 cleanup()
@@ -270,13 +284,13 @@ if is_enabled CONFIG_KALLSYMS; then
 	strip_debug=1
 
 	sysmap_and_kallsyms .tmp_vmlinux1
-	size1=$(${CONFIG_SHELL} "${srctree}/scripts/file-size.sh" ${kallsymso})
+	[ -v dry_run ] || size1=$(${CONFIG_SHELL} "${srctree}/scripts/file-size.sh" ${kallsymso})
 
 	vmlinux_link .tmp_vmlinux2
 	sysmap_and_kallsyms .tmp_vmlinux2
-	size2=$(${CONFIG_SHELL} "${srctree}/scripts/file-size.sh" ${kallsymso})
+	[ -v dry_run ] || size2=$(${CONFIG_SHELL} "${srctree}/scripts/file-size.sh" ${kallsymso})
 
-	if [ $size1 -ne $size2 ] || [ -n "${KALLSYMS_EXTRA_PASS}" ]; then
+	if [ ! -v dry_run ] || [ $size1 -ne $size2 ] || [ -n "${KALLSYMS_EXTRA_PASS}" ]; then
 		vmlinux_link .tmp_vmlinux3
 		sysmap_and_kallsyms .tmp_vmlinux3
 	fi
@@ -289,7 +303,7 @@ vmlinux_link vmlinux
 # fill in BTF IDs
 if is_enabled CONFIG_DEBUG_INFO_BTF && is_enabled CONFIG_BPF; then
 	info BTFIDS vmlinux
-	${RESOLVE_BTFIDS} vmlinux
+	run ${RESOLVE_BTFIDS} vmlinux
 fi
 
 mksysmap vmlinux System.map
@@ -303,7 +317,7 @@ if is_enabled CONFIG_BUILDTIME_TABLE_SORT; then
 fi
 
 # step a (see comment above)
-if is_enabled CONFIG_KALLSYMS; then
+if [ ! -v dry_run ] && is_enabled CONFIG_KALLSYMS; then
 	if ! cmp -s System.map "${kallsyms_sysmap}"; then
 		echo >&2 Inconsistent kallsyms data
 		echo >&2 'Try "make KALLSYMS_EXTRA_PASS=1" as a workaround'
-- 
2.34.1

[RFC PATCH 09/11] kbuild: simplify commands in --dry-run mode

[Vegard Nossum]

- $filechk is used to check if a file is up to date.

- $cmd includes logic for echoing commands and deleting intermediate
  files on interrupt. Skip all of that in --dry-run mode and just execute
  the command.

- $cmd_and_savecmd executes the command and echoes it into .<target>.cmd.

- $if_changed_dep executes the command if any dependencies have changed.

- $cmd_and_fixdep executes the command and updates .<target>.cmd.

Skip all of that in --dry-run mode and just execute the command.

Signed-off-by: Vegard Nossum <vegard.nossum@oracle.com>
---
 scripts/Kbuild.include | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include
index ed8a7493524b2..a1ef3b1828bb3 100644
--- a/scripts/Kbuild.include
+++ b/scripts/Kbuild.include
@@ -94,6 +94,7 @@ kecho := $($(quiet)kecho)
 # - If no file exist it is created
 # - If the content differ the new file is used
 # - If they are equal no change, and no timestamp update
+ifndef dry_run
 define filechk
 	$(check-FORCE)
 	$(Q)set -e;						\
@@ -105,6 +106,14 @@ define filechk
 		mv -f $(tmp-target) $@;				\
 	fi
 endef
+else
+# simplify and write the output directly if we're just printing
+# the commands
+define filechk
+	mkdir -p $(dir $@)
+	{ $(filechk_$(1)); } > $@
+endef
+endif
 
 ###
 # Shorthand for $(Q)$(MAKE) -f scripts/Makefile.build obj=
@@ -149,8 +158,13 @@ delete-on-interrupt = \
 		$(foreach sig, HUP INT QUIT TERM PIPE, \
 			trap 'rm -f $@; trap - $(sig); kill -s $(sig) $$$$' $(sig);))
 
+ifndef dry_run
 # print and execute commands
 cmd = @$(if $(cmd_$(1)),set -e; $($(quiet)log_print) $(delete-on-interrupt) $(cmd_$(1)),:)
+else
+# just execute (...which will actually "just print" with make -n)
+cmd = @$(if $(cmd_$(1)),$(cmd_$(1)),)
+endif
 
 ###
 # if_changed      - execute command if any prerequisite is newer than
@@ -196,17 +210,30 @@ if-changed-cond = $(newer-prereqs)$(cmd-check)$(check-FORCE)
 # Execute command if command has changed or prerequisite(s) are updated.
 if_changed = $(if $(if-changed-cond),$(cmd_and_savecmd),@:)
 
+ifndef dry_run
 cmd_and_savecmd =                                                            \
 	$(cmd);                                                              \
 	printf '%s\n' 'savedcmd_$@ := $(make-cmd)' > $(dot-target).cmd
+else
+cmd_and_savecmd = $(cmd)
+endif
 
+ifndef dry_run
 # Execute the command and also postprocess generated .d dependencies file.
 if_changed_dep = $(if $(if-changed-cond),$(cmd_and_fixdep),@:)
+else
+# Just execute the command directly
+if_changed_dep = $(cmd)
+endif
 
+ifndef dry_run
 cmd_and_fixdep =                                                             \
 	$(cmd);                                                              \
 	scripts/basic/fixdep $(depfile) $@ '$(make-cmd)' > $(dot-target).cmd;\
 	rm -f $(depfile)
+else
+cmd_and_fixdep = $(cmd)
+endif
 
 # Usage: $(call if_changed_rule,foo)
 # Will check if $(cmd_foo) or any of the prerequisites changed,
-- 
2.34.1

[RFC PATCH 10/11] kbuild: don't test for file presence in --dry-run mode

[Vegard Nossum]

I'm not really sure if this is correct as I'm not sure under which
circumstances the files tested for with $(wildcard) would NOT be
present. I'm not even sure if this is the right approach to take.

However, it _should_ keep working the same as before for regular
'make' invocations and is necessary for 'make --dry-run' to work.

Signed-off-by: Vegard Nossum <vegard.nossum@oracle.com>
---
 scripts/Makefile.modpost | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/scripts/Makefile.modpost b/scripts/Makefile.modpost
index 65f2bdc702369..139fa0b087b6d 100644
--- a/scripts/Makefile.modpost
+++ b/scripts/Makefile.modpost
@@ -99,7 +99,7 @@ modpost-args += -t $(addprefix -u , $(ksym-wl))
 modpost-deps += $(ksym-wl)
 endif
 
-ifeq ($(wildcard vmlinux.o),)
+ifeq ($(or $(wildcard vmlinux.o), $(dry_run)),)
 missing-input := vmlinux.o
 output-symdump := modules-only.symvers
 else
@@ -119,7 +119,7 @@ include $(kbuild-file)
 
 output-symdump := $(KBUILD_EXTMOD)/Module.symvers
 
-ifeq ($(wildcard Module.symvers),)
+ifeq ($(or $(wildcard Module.symvers), $(dry_run)),)
 missing-input := Module.symvers
 else
 modpost-args += -i Module.symvers
-- 
2.34.1

[RFC PATCH 11/11] kbuild: suppress echoing of commands in --dry-run mode

[Vegard Nossum]

If the user ran 'make -n' then we will already print all commands.

Signed-off-by: Vegard Nossum <vegard.nossum@oracle.com>
---
 Makefile | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/Makefile b/Makefile
index d08ade5791c2e..a1a3e96a10ea2 100644
--- a/Makefile
+++ b/Makefile
@@ -96,9 +96,10 @@ ifneq ($(findstring 1, $(KBUILD_VERBOSE)),)
   Q =
 endif
 
-# If the user is running make -s (silent mode), suppress echoing of
-# commands
-ifneq ($(findstring s,$(firstword -$(MAKEFLAGS))),)
+# If the user is running make -s (silent mode) or -n (dry run mode),
+# suppress echoing of commands
+ifneq (,$(or $(findstring s,$(firstword -$(MAKEFLAGS))), \
+	$(findstring n,$(firstword -$(MAKEFLAGS)))))
 quiet=silent_
 override KBUILD_VERBOSE :=
 endif
-- 
2.34.1

Re: [RFC PATCH 00/11] output a valid shell script when running 'make -n'

[Vegard Nossum]

Hi,

I didn't receive a single comment on this patch series since I submitted
it a month ago, but I understand it's been busy with conferences and the
merge window.

I've rebased it on latest mainline (including the kbuild-6.12 merge) and
there's just one tiny trivial conflict. Can/should I wait for -rc1 and
resubmit it for inclusion then?

Thanks,


Vegard

On 19/08/2024 18:02, Vegard Nossum wrote:
> This patch series lets 'make -n' output a shell script that can be
> used to build the kernel without any further use of make. For example:
> 
>      make defconfig
> 
>      # ensure some build prerequisites are built
>      make prepare
> 
>      # generate build script
>      make -n | tee build.sh
> 
>      # excecute build script
>      bash -eux build.sh
> 
> The purpose of this is to take a step towards defeating the insertion of
> backdoors at build time (see [1]). Some of the benefits of separating the
> build script from the build system are:
> 
>   - we can invoke make in a restricted environment (e.g. mostly read-only
>     kernel tree),
> 
>   - we have an audit log of the exact commands that run during the build
>     process; although it's true that the build script wouldn't be useful
>     for either production or development builds (as it doesn't support
>     incremental rebuilds or parallel builds), it would allow you to
>     rebuild an existing kernel and compare the resulting binary for
>     discrepancies to the original build,
> 
>   - the audit log can be stored (e.g. in git) and changes to it over time
>     can themselves be audited (e.g. by looking at diffs),
> 
>   - there's a lot fewer places to hide malicious code in a straight-line
>     shell script that makes minimal use of variables and helper functions.
>     You also cannot inject fragments of Makefile code through environment
>     variables (again, see [1]).
> 
> Alternative ways to achieve some of the same things would be:
> 
>   - the existing compile_commands.json infrastructure; unfortunately this
>     does not include most of the steps performed during a build (such as
>     linking vmlinux) and does not really allow you to reproduce/verify the
>     full build,
> 
>   - simply running something like strace -f -e trace=execve make; however,
>     this also does not result in something that can be easily played back;
>     at the very least it would need to be heavily filtered and processed
>     to account for data passed in environment variables and things like
>     temporary files used by the compiler.
> 
> This implementation works as follows:
> 
>   - 'make -n' (AKA --dry-run) by default prints out the commands that make
>     runs; this output is modified to be usable as a shell script,
> 
>   - we output 'make() { :; }' at the start of the script in order to make
>     all 'make' invocations in the resulting build script no-ops (GNU Make
>     will actually execute -- and print -- all recipe lines that include
>     $(MAKE), even when invoked with -n).
> 
>   - we simplify the makefile rules in some cases to make the shell script
>     more readable; for example, we don't need the logic that extracts
>     dependencies from .c files (since that is only used by 'make' itself
>     when determining what to rebuild) or the logic that generates .cmd
>     files,
> 
> This patch is WIP and may not produce a working shell script in all
> circumstances. For example, while plain 'make -n' works for me, other
> make targets (e.g. 'make -n htmldocs') are not at all guaranteed to
> produce meaningful output; certain kernel configs may also not work,
> especially those that rely on external tools like e.g. Rust.
> 
> [1]: https://www.openwall.com/lists/oss-security/2024/04/17/3
> [2]: https://www.gnu.org/software/make/manual/make.html#Testing-Flags
> 
> 
> Vegard
> 
> ---
> 
> Vegard Nossum (11):
>    kbuild: ignore .config rule for make --always-make
>    kbuild: document some prerequisites
>    kbuild: pass KERNELVERSION and LOCALVERSION explicitly to
>      setlocalversion
>    kbuild: don't execute .ko recipe in --dry-run mode
>    kbuild: execute modules.order recipe in --dry-run mode
>    kbuild: set $dry_run when running in --dry-run mode
>    kbuild: define 'make' as a no-op in --dry-run mode
>    kbuild: make link-vmlinux.sh respect $dry_run
>    kbuild: simplify commands in --dry-run mode
>    kbuild: don't test for file presence in --dry-run mode
>    kbuild: suppress echoing of commands in --dry-run mode
> 
>   Makefile                          | 28 +++++++++++++++++---
>   arch/x86/boot/compressed/Makefile |  6 +++++
>   scripts/Kbuild.include            | 27 +++++++++++++++++++
>   scripts/Makefile.build            |  2 +-
>   scripts/Makefile.modfinal         |  9 +++++--
>   scripts/Makefile.modpost          |  8 ++++--
>   scripts/Makefile.vmlinux          | 22 ++++++++++++++--
>   scripts/Makefile.vmlinux_o        |  3 +++
>   scripts/link-vmlinux.sh           | 44 ++++++++++++++++++++-----------
>   9 files changed, 123 insertions(+), 26 deletions(-)
> 

Re: [RFC PATCH 00/11] output a valid shell script when running 'make -n'

[Nicolas Schier]

[-- Attachment #1: Type: text/plain, Size: 5686 bytes --]

On Mon, Aug 19, 2024 at 06:02:57PM +0200 Vegard Nossum wrote:
> This patch series lets 'make -n' output a shell script that can be
> used to build the kernel without any further use of make. For example:
> 
>     make defconfig
> 
>     # ensure some build prerequisites are built
>     make prepare
> 
>     # generate build script
>     make -n | tee build.sh
> 
>     # excecute build script
>     bash -eux build.sh
> 
> The purpose of this is to take a step towards defeating the insertion of
> backdoors at build time (see [1]). Some of the benefits of separating the
> build script from the build system are:
> 
>  - we can invoke make in a restricted environment (e.g. mostly read-only
>    kernel tree),
> 
>  - we have an audit log of the exact commands that run during the build
>    process; although it's true that the build script wouldn't be useful
>    for either production or development builds (as it doesn't support
>    incremental rebuilds or parallel builds), it would allow you to
>    rebuild an existing kernel and compare the resulting binary for
>    discrepancies to the original build,
> 
>  - the audit log can be stored (e.g. in git) and changes to it over time
>    can themselves be audited (e.g. by looking at diffs),
> 
>  - there's a lot fewer places to hide malicious code in a straight-line
>    shell script that makes minimal use of variables and helper functions.
>    You also cannot inject fragments of Makefile code through environment
>    variables (again, see [1]).
> 
> Alternative ways to achieve some of the same things would be:
> 
>  - the existing compile_commands.json infrastructure; unfortunately this
>    does not include most of the steps performed during a build (such as
>    linking vmlinux) and does not really allow you to reproduce/verify the
>    full build,
> 
>  - simply running something like strace -f -e trace=execve make; however,
>    this also does not result in something that can be easily played back;
>    at the very least it would need to be heavily filtered and processed
>    to account for data passed in environment variables and things like
>    temporary files used by the compiler.
> 
> This implementation works as follows:
> 
>  - 'make -n' (AKA --dry-run) by default prints out the commands that make
>    runs; this output is modified to be usable as a shell script,
> 
>  - we output 'make() { :; }' at the start of the script in order to make
>    all 'make' invocations in the resulting build script no-ops (GNU Make
>    will actually execute -- and print -- all recipe lines that include
>    $(MAKE), even when invoked with -n).
> 
>  - we simplify the makefile rules in some cases to make the shell script
>    more readable; for example, we don't need the logic that extracts
>    dependencies from .c files (since that is only used by 'make' itself
>    when determining what to rebuild) or the logic that generates .cmd
>    files,
> 
> This patch is WIP and may not produce a working shell script in all
> circumstances. For example, while plain 'make -n' works for me, other
> make targets (e.g. 'make -n htmldocs') are not at all guaranteed to
> produce meaningful output; certain kernel configs may also not work,
> especially those that rely on external tools like e.g. Rust.

Thanks for this patch set and all the thoughts laid out here in detail,
especially for the write-up in [1], too!

I think it is a good idea to work towards hardening the build system against
known and difficult to spot attacks.  As the patch set integration needs a
complete 'make -n' script (at least for a "simple", defined config) to be
successful, I expect that it might become quite some work and patience, but I
think it is a meaningful goal.

In order to prevent "degradation" of Make rules after a possible integration,
we need some (automated) testers, otherwise we will loose all the efforts again.

Please give me yet some days for a first rough round through the patches.

Kind regards
Nicolas



> 
> [1]: https://www.openwall.com/lists/oss-security/2024/04/17/3
> [2]: https://www.gnu.org/software/make/manual/make.html#Testing-Flags
> 
> 
> Vegard
> 
> ---
> 
> Vegard Nossum (11):
>   kbuild: ignore .config rule for make --always-make
>   kbuild: document some prerequisites
>   kbuild: pass KERNELVERSION and LOCALVERSION explicitly to
>     setlocalversion
>   kbuild: don't execute .ko recipe in --dry-run mode
>   kbuild: execute modules.order recipe in --dry-run mode
>   kbuild: set $dry_run when running in --dry-run mode
>   kbuild: define 'make' as a no-op in --dry-run mode
>   kbuild: make link-vmlinux.sh respect $dry_run
>   kbuild: simplify commands in --dry-run mode
>   kbuild: don't test for file presence in --dry-run mode
>   kbuild: suppress echoing of commands in --dry-run mode
> 
>  Makefile                          | 28 +++++++++++++++++---
>  arch/x86/boot/compressed/Makefile |  6 +++++
>  scripts/Kbuild.include            | 27 +++++++++++++++++++
>  scripts/Makefile.build            |  2 +-
>  scripts/Makefile.modfinal         |  9 +++++--
>  scripts/Makefile.modpost          |  8 ++++--
>  scripts/Makefile.vmlinux          | 22 ++++++++++++++--
>  scripts/Makefile.vmlinux_o        |  3 +++
>  scripts/link-vmlinux.sh           | 44 ++++++++++++++++++++-----------
>  9 files changed, 123 insertions(+), 26 deletions(-)
> 
> -- 
> 2.34.1
> 

-- 
epost|xmpp: nicolas@fjasle.eu          irc://oftc.net/nsc
↳ gpg: 18ed 52db e34f 860e e9fb  c82b 7d97 0932 55a0 ce7f
     -- frykten for herren er opphav til kunnskap --

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [RFC PATCH 01/11] kbuild: ignore .config rule for make --always-make

[Nicolas Schier]

On Mon, Aug 19, 2024 at 06:02:58PM +0200 Vegard Nossum wrote:
> Before this patch, using 'make --always-make' would always result in the
> error message about the missing .config being displayed.
> 
> Detect the -B/--always-make flag and leave this rule out, which allows the
> rest of the build to proceed. See [1] for an explanation of this particular
> construction.

Nice catch, thanks.

> 
> [1]: https://www.gnu.org/software/make/manual/make.html#Testing-Flags
> 
> Signed-off-by: Vegard Nossum <vegard.nossum@oracle.com>
> ---
>  Makefile | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/Makefile b/Makefile
> index 44c02a6f60a14..f09c036daf2f5 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -757,6 +757,10 @@ ifdef may-sync-config
>  # because some architectures define CROSS_COMPILE there.
>  include include/config/auto.conf.cmd
>  
> +ifeq (,$(findstring B,$(firstword -$(MAKEFLAGS))))

As we still also support make v3.80 to v4.0, please use $(short-opts)
defined around line 27.

> +# This is a dummy target, only meant as a help for the user invoking make.
> +# We don't want it to take effect when running 'make --always-make', since
> +# that renders the --always-make option effectively useless.
>  $(KCONFIG_CONFIG):
>  	@echo >&2 '***'
>  	@echo >&2 '*** Configuration file "$@" not found!'
> @@ -765,6 +769,7 @@ $(KCONFIG_CONFIG):
>  	@echo >&2 '*** "make menuconfig" or "make xconfig").'
>  	@echo >&2 '***'
>  	@/bin/false
> +endif
>  
>  # The actual configuration files used during the build are stored in
>  # include/generated/ and include/config/. Update them if .config is newer than
> -- 
> 2.34.1
> 

Re: [RFC PATCH 02/11] kbuild: document some prerequisites

[Nicolas Schier]

On Mon, Aug 19, 2024 at 06:02:59PM +0200 Vegard Nossum wrote:
> When running 'make --dry-run', make will invoke itself recursively for
> recipes using $(MAKE), but otherwise not execute the other commands in
> a recipe.
> 
> However, if a prerequisite is missing and 'make' does not how to create it
> (which will be the case when running 'make -n'), it will complain with an
> error message like this:
> 
>   $ make -n
>   ...
>   make -f ./scripts/Makefile.modpost
>   make[1]: *** No rule to make target 'modules.order', needed by 'modules-only.symvers'.  Stop.
>   make: *** [Makefile:1868: modpost] Error 2
> 
> In this case, the top-level makefile has reached a recipe that ran 'make'
> recursively on scripts/Makefile.modpost, which itself has a rule with
> modules.order as a prerequisite. Since the file doesn't exist, and make
> doesn't know how to create it, it errors out.
> 
> We can document such prerequisites (which are expected to be created by
> the parent Makefile) by adding them to the PHONY list of each respective
> Makefile.
> 
> Signed-off-by: Vegard Nossum <vegard.nossum@oracle.com>
> ---
>  arch/x86/boot/compressed/Makefile | 6 ++++++
>  scripts/Makefile.modfinal         | 5 +++++
>  scripts/Makefile.modpost          | 4 ++++
>  scripts/Makefile.vmlinux          | 7 +++++++
>  scripts/Makefile.vmlinux_o        | 3 +++
>  5 files changed, 25 insertions(+)
> 
> diff --git a/arch/x86/boot/compressed/Makefile b/arch/x86/boot/compressed/Makefile
> index f2051644de943..ccef6f0e04bc7 100644
> --- a/arch/x86/boot/compressed/Makefile
> +++ b/arch/x86/boot/compressed/Makefile
> @@ -80,6 +80,9 @@ quiet_cmd_voffset = VOFFSET $@
>  
>  targets += ../voffset.h
>  
> +# We don't know how to build this

A comment for documentation is a good idea, but I think this one is not very
helpful to those who don't know the patch description.  What about something
like this?

# Provided by a recursive-make predecessor


Kind regards
Nicolas

Re: [RFC PATCH 03/11] kbuild: pass KERNELVERSION and LOCALVERSION explicitly to setlocalversion

[Nicolas Schier]

On Mon, Aug 19, 2024 at 06:03:00PM +0200 Vegard Nossum wrote:
> These environment variables are passed when invoking 'make', but if
> running 'make -n' we need to pass them explicitly so they become part
> of the printed command.
> 
> Signed-off-by: Vegard Nossum <vegard.nossum@oracle.com>
> ---
>  Makefile | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 

Reviewed-by: Nicolas Schier <nicolas@fjasle.eu>

Re: [RFC PATCH 04/11] kbuild: don't execute .ko recipe in --dry-run mode

[Nicolas Schier]

On Mon, Aug 19, 2024 at 06:03:01PM +0200 Vegard Nossum wrote:
> Prefixing a line in a make recipe with + makes that command execute even
> in --dry-run mode. We don't need that here; remove it.

In case somebody else also wants to know why make behaves that way:
https://www.gnu.org/software/make/manual/make.html#MAKE-Variable

> 
> Fixes: 5f9ae91f7c0d ("kbuild: Build kernel module BTFs if BTF is enabled and pahole supports it")
> Cc: Andrii Nakryiko <andrii@kernel.org>
> Cc: Alexei Starovoitov <ast@kernel.org>
> Signed-off-by: Vegard Nossum <vegard.nossum@oracle.com>
> ---
>  scripts/Makefile.modfinal | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/scripts/Makefile.modfinal b/scripts/Makefile.modfinal
> index 696888f0a0bde..2679304f158ad 100644
> --- a/scripts/Makefile.modfinal
> +++ b/scripts/Makefile.modfinal
> @@ -60,9 +60,9 @@ if_changed_except = $(if $(call newer_prereqs_except,$(2))$(cmd-check),      \
>  
>  # Re-generate module BTFs if either module's .ko or vmlinux changed
>  %.ko: %.o %.mod.o scripts/module.lds $(and $(CONFIG_DEBUG_INFO_BTF_MODULES),$(KBUILD_BUILTIN),vmlinux) FORCE
> -	+$(call if_changed_except,ld_ko_o,vmlinux)
> +	$(call if_changed_except,ld_ko_o,vmlinux)
>  ifdef CONFIG_DEBUG_INFO_BTF_MODULES
> -	+$(if $(newer-prereqs),$(call cmd,btf_ko))
> +	$(if $(newer-prereqs),$(call cmd,btf_ko))

I assumed that some ld might evaluate the jobserver variables (MAKEFLAGS), but
following the discussion in [1], this does not seem to be relevant.

Can you at a note to the commit message, that there is no need to run $(LD) or
$(PAHOLE) in parallel here?

[1]: https://lore.kernel.org/all/20220616104541.16289-1-jslaby@suse.cz/#t

Re: [RFC PATCH 05/11] kbuild: execute modules.order recipe in --dry-run mode

[Nicolas Schier]

On Mon, Aug 19, 2024 at 06:03:02PM +0200 Vegard Nossum wrote:
> modules.order is read by scripts/Makefile.modfinal to determine which
> modules to build, so we need this recipe to execute if we want to be able
> to output the recipes for building modules in dry-run mode.
> 
> Signed-off-by: Vegard Nossum <vegard.nossum@oracle.com>
> ---
>  scripts/Makefile.build | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/scripts/Makefile.build b/scripts/Makefile.build
> index efacca63c8976..34f86dced67f3 100644
> --- a/scripts/Makefile.build
> +++ b/scripts/Makefile.build
> @@ -417,7 +417,7 @@ cmd_gen_order = { $(foreach m, $(real-prereqs), \
>  	> $@
>  
>  $(obj)/modules.order: $(obj-m) FORCE
> -	$(call if_changed,gen_order)
> +	+$(call if_changed,gen_order)

As it is a bit hacky to mark an always sequential command as 'recursive' [1] in
order to circumvent the '--dry-run', I'd like to see a comment in the code,
too.

[1]: https://www.gnu.org/software/make/manual/make.html#MAKE-Variable

Re: [RFC PATCH 06/11] kbuild: set $dry_run when running in --dry-run mode

[Nicolas Schier]

On Mon, Aug 19, 2024 at 06:03:03PM +0200 Vegard Nossum wrote:
> Add a convenience variable that allows us to use 'ifdef dry_run...endif'
> in Makefiles or '[ -v dry_run ]' in shell scripts to test whether make
> was invoked with '-n'.
> 
> See [1] for an explanation of this particular construction.
> 
> [1]: https://www.gnu.org/software/make/manual/make.html#Testing-Flags
> 
> Signed-off-by: Vegard Nossum <vegard.nossum@oracle.com>
> ---
>  Makefile | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/Makefile b/Makefile
> index 58f3843ccfac6..953951157ec92 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -22,6 +22,11 @@ $(if $(filter __%, $(MAKECMDGOALS)), \
>  PHONY := __all
>  __all:
>  
> +# Was make invoked with --dry-run/-n? Record this in a convenience variable.
> +ifeq (n,$(findstring n,$(firstword -$(MAKEFLAGS))))
> +export dry_run := 1
> +endif

As in patch 1: 
As we still also support make v3.80 to v4.0, please use $(short-opts)
defined around line 27.

> +
>  # We are using a recursive build, so we need to do a little thinking
>  # to get the ordering right.
>  #
> -- 
> 2.34.1
> 

I will have a look at the remaining patches within a few days.

Thanks and kind regards,
Nicolas

Re: [RFC PATCH 01/11] kbuild: ignore .config rule for make --always-make

[Miguel Ojeda]

On Sat, Nov 2, 2024 at 10:08 PM Nicolas Schier <nicolas@fjasle.eu> wrote:
>
> As we still also support make v3.80 to v4.0, please use $(short-opts)
> defined around line 27.

We moved to 4.0 in 5f99665ee8f4 ("kbuild: raise the minimum GNU Make
requirement to 4.0") -- or do you mean something else / am I missing
something?

Thanks!

Cheers,
Miguel

Re: [RFC PATCH 01/11] kbuild: ignore .config rule for make --always-make

[Nicolas Schier]

[-- Attachment #1: Type: text/plain, Size: 471 bytes --]

On Sat, Nov 02, 2024 at 10:39:07PM +0100 Miguel Ojeda wrote:
> On Sat, Nov 2, 2024 at 10:08 PM Nicolas Schier <nicolas@fjasle.eu> wrote:
> >
> > As we still also support make v3.80 to v4.0, please use $(short-opts)
> > defined around line 27.
> 
> We moved to 4.0 in 5f99665ee8f4 ("kbuild: raise the minimum GNU Make
> requirement to 4.0") -- or do you mean something else / am I missing
> something?

oh yes, thanks. I missed that,

Kind regards
Nicolas

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [RFC PATCH 07/11] kbuild: define 'make' as a no-op in --dry-run mode

[Nicolas Schier]

On Mon, Aug 19, 2024 at 06:03:04PM +0200, Vegard Nossum wrote:
> Output 'make() { :; }' at the start of the script in order to make
> all 'make' invocations in the resulting build script no-ops (GNU Make
> will actually execute -- and print -- all recipe lines that include
> $(MAKE), even when invoked with -n).
> 
> Signed-off-by: Vegard Nossum <vegard.nossum@oracle.com>
> ---
>  Makefile | 6 ++++++
>  1 file changed, 6 insertions(+)

Reviewed-by: Nicolas Schier <nicolas@fjasle.eu>

Re: [RFC PATCH 08/11] kbuild: make link-vmlinux.sh respect $dry_run

[Nicolas Schier]

On Mon, Aug 19, 2024 at 06:03:05PM +0200, Vegard Nossum wrote:
> Make link-vmlinux.sh print the commands it wants to run instead of
> actually running them when $dry_run is defined.

Do we need link-vmlinux.sh to show it's commands?  Couldn't we just show
the call of link-vmlinux.sh instead (and replace/hide the '+' prefix if
'dry_run' is set)?  Why not?  (I guess, you found good reasons.  Can you
please add them to the commit message?)

> 
> This is needed in order for make -n/--dry-run to output a complete
> build script.
> 
> Signed-off-by: Vegard Nossum <vegard.nossum@oracle.com>
> ---
>  scripts/Makefile.vmlinux | 15 ++++++++++++--
>  scripts/link-vmlinux.sh  | 44 ++++++++++++++++++++++++++--------------
>  2 files changed, 42 insertions(+), 17 deletions(-)
> 
> diff --git a/scripts/Makefile.vmlinux b/scripts/Makefile.vmlinux
> index 10d80e07f945c..1fe0faab52889 100644
> --- a/scripts/Makefile.vmlinux
> +++ b/scripts/Makefile.vmlinux
> @@ -1,5 +1,12 @@
>  # SPDX-License-Identifier: GPL-2.0-only
>  
> +# there doesn't seem to be a straightforward way for 'make -n' to output
> +# one command while actually executing another, so we resort to this hack
> +# where we set an environment variable differently depending on whether
> +# we are executing from 'make' or executing from the 'make -n' log.
> +LINK_VMLINUX=scripts/link-vmlinux.sh
> +export LINK_VMLINUX
> +
>  PHONY := __default
>  __default: vmlinux
>  
> @@ -29,7 +36,8 @@ ARCH_POSTLINK := $(wildcard $(srctree)/arch/$(SRCARCH)/Makefile.postlink)
>  
>  # Final link of vmlinux with optional arch pass after final link
>  cmd_link_vmlinux =							\
> -	$< "$(LD)" "$(KBUILD_LDFLAGS)" "$(LDFLAGS_vmlinux)";		\
> +	"$${LINK_VMLINUX}"						\
> +	"$(LD)" "$(KBUILD_LDFLAGS)" "$(LDFLAGS_vmlinux)";		\
>  	$(if $(ARCH_POSTLINK), $(MAKE) -f $(ARCH_POSTLINK) $@, true)
>  
>  # We don't know how to build these
> @@ -37,7 +45,10 @@ PHONY += vmlinux.o
>  PHONY += $(KBUILD_LDS)
>  
>  targets += vmlinux
> -vmlinux: scripts/link-vmlinux.sh vmlinux.o $(KBUILD_LDS) FORCE
> +vmlinux: $(LINK_VMLINUX) vmlinux.o $(KBUILD_LDS) FORCE
> +ifdef dry_run
> +	@LINK_VMLINUX=/bin/true
> +endif
>  	+$(call if_changed_dep,link_vmlinux)
>  
>  # Add FORCE to the prequisites of a target to force it to be always rebuilt.
> diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
> index f7b2503cdba95..fd32e48a8a455 100755
> --- a/scripts/link-vmlinux.sh
> +++ b/scripts/link-vmlinux.sh
> @@ -32,6 +32,17 @@ LD="$1"
>  KBUILD_LDFLAGS="$2"
>  LDFLAGS_vmlinux="$3"
>  
> +# If $dry_run is set, just echo the command instead of executing it.
> +run()
> +{
> +	if [ ! -v dry_run ]
> +	then
> +		eval "$1"
> +	else
> +		echo "$1"
> +	fi
> +}
> +
>  is_enabled() {
>  	grep -q "^$1=y" include/config/auto.conf
>  }
> @@ -40,7 +51,10 @@ is_enabled() {
>  # Will be supressed by "make -s"
>  info()
>  {
> -	printf "  %-7s %s\n" "${1}" "${2}"
> +	if [ ! -v dry_run ]
> +	then
> +		printf "  %-7s %s\n" "${1}" "${2}"
> +	fi
>  }
>  
>  # Link of vmlinux

Do you have some thoughts about the

     if is_enabled ...

lines?  Do we need to defer the evaluation of include/config/auto.conf,
too?


> @@ -97,10 +111,10 @@ vmlinux_link()
>  		ldflags="${ldflags} ${wl}-Map=${output}.map"
>  	fi
>  
> -	${ld} ${ldflags} -o ${output}					\
> +	run "${ld} ${ldflags} -o ${output}				\
>  		${wl}--whole-archive ${objs} ${wl}--no-whole-archive	\
>  		${wl}--start-group ${libs} ${wl}--end-group		\
> -		${kallsymso} ${btf_vmlinux_bin_o} ${ldlibs}
> +		${kallsymso} ${btf_vmlinux_bin_o} ${ldlibs}"
>  }
>  
>  # generate .BTF typeinfo from DWARF debuginfo
> @@ -129,8 +143,8 @@ gen_btf()
>  	# deletes all symbols including __start_BTF and __stop_BTF, which will
>  	# be redefined in the linker script. Add 2>/dev/null to suppress GNU
>  	# objcopy warnings: "empty loadable segment detected at ..."
> -	${OBJCOPY} --only-section=.BTF --set-section-flags .BTF=alloc,readonly \
> -		--strip-all ${1} "${btf_data}" 2>/dev/null
> +	run "${OBJCOPY} --only-section=.BTF --set-section-flags .BTF=alloc,readonly \
> +		--strip-all ${1} "${btf_data}" 2>/dev/null"

                                 ^^^^^^^^^^^^^
Backslashes to keep ${btf_data} quoted are missing.

>  	# Change e_type to ET_REL so that it can be used to link final vmlinux.
>  	# GNU ld 2.35+ and lld do not allow an ET_EXEC input.
>  	if is_enabled CONFIG_CPU_BIG_ENDIAN; then

printf "${et_rel}" | dd of="${btf_data}" conv=notrunc bs=1 seek=16 status=none

'dd' should probably be called via 'run'.


> @@ -161,11 +175,11 @@ kallsyms()
>  	fi
>  
>  	info KSYMS "${2}.S"
> -	scripts/kallsyms ${kallsymopt} "${1}" > "${2}.S"
> +	run "scripts/kallsyms ${kallsymopt} \"${1}\" > \"${2}.S\""
>  
>  	info AS "${2}.o"
> -	${CC} ${NOSTDINC_FLAGS} ${LINUXINCLUDE} ${KBUILD_CPPFLAGS} \
> -	      ${KBUILD_AFLAGS} ${KBUILD_AFLAGS_KERNEL} -c -o "${2}.o" "${2}.S"
> +	run "${CC} ${NOSTDINC_FLAGS} ${LINUXINCLUDE} ${KBUILD_CPPFLAGS} \
> +	      ${KBUILD_AFLAGS} ${KBUILD_AFLAGS_KERNEL} -c -o \"${2}.o\" \"${2}.S\""
>  
>  	kallsymso=${2}.o
>  }
> @@ -184,12 +198,12 @@ sysmap_and_kallsyms()
>  mksysmap()
>  {
>  	info NM ${2}
> -	${NM} -n "${1}" | sed -f "${srctree}/scripts/mksysmap" > "${2}"
> +	run "${NM} -n \"${1}\" | sed -f \"${srctree}/scripts/mksysmap\" > \"${2}\""
>  }
>  
>  sorttable()
>  {
> -	${objtree}/scripts/sorttable ${1}
> +	run "${objtree}/scripts/sorttable ${1}"
>  }
>  
>  cleanup()

I know, 'link-vmlinux.sh clean' will not by called by 'make --dry-run
clean' (as there is no '+' prefix in the call in Makefile), but for
consistency, I'd like to see also cleanup() be prepared for handling
dry_run correctly.


Can you please move patch 11 (resetting of KBUILD_VERBOSE if '--dry-run'
or '--silent' is given) before this one?


Does '${MAKE} ...' also need to be called via 'run'?

> @@ -270,13 +284,13 @@ if is_enabled CONFIG_KALLSYMS; then
>  	strip_debug=1
>  
>  	sysmap_and_kallsyms .tmp_vmlinux1
> -	size1=$(${CONFIG_SHELL} "${srctree}/scripts/file-size.sh" ${kallsymso})
> +	[ -v dry_run ] || size1=$(${CONFIG_SHELL} "${srctree}/scripts/file-size.sh" ${kallsymso})
>  
>  	vmlinux_link .tmp_vmlinux2
>  	sysmap_and_kallsyms .tmp_vmlinux2
> -	size2=$(${CONFIG_SHELL} "${srctree}/scripts/file-size.sh" ${kallsymso})
> +	[ -v dry_run ] || size2=$(${CONFIG_SHELL} "${srctree}/scripts/file-size.sh" ${kallsymso})
>  
> -	if [ $size1 -ne $size2 ] || [ -n "${KALLSYMS_EXTRA_PASS}" ]; then
> +	if [ ! -v dry_run ] || [ $size1 -ne $size2 ] || [ -n "${KALLSYMS_EXTRA_PASS}" ]; then
>  		vmlinux_link .tmp_vmlinux3
>  		sysmap_and_kallsyms .tmp_vmlinux3
>  	fi
> @@ -289,7 +303,7 @@ vmlinux_link vmlinux
>  # fill in BTF IDs
>  if is_enabled CONFIG_DEBUG_INFO_BTF && is_enabled CONFIG_BPF; then
>  	info BTFIDS vmlinux
> -	${RESOLVE_BTFIDS} vmlinux
> +	run ${RESOLVE_BTFIDS} vmlinux

Quotes are missing, as run only evaluated its first argument.

>  fi
>  
>  mksysmap vmlinux System.map
> @@ -303,7 +317,7 @@ if is_enabled CONFIG_BUILDTIME_TABLE_SORT; then

I am sceptical about

	if ! sorttable vmlinux

and similar.  With dry_run being set, sorttable shows the call of
'sorttable' and always returns success.  In effect, the resulting output
shell build script will miss such error detection.

I think it would be really good, if we could skip patching
link-vmlinux.sh but show its call instead.

Kind regards,
Nicolas

>  fi
>  
>  # step a (see comment above)
> -if is_enabled CONFIG_KALLSYMS; then
> +if [ ! -v dry_run ] && is_enabled CONFIG_KALLSYMS; then
>  	if ! cmp -s System.map "${kallsyms_sysmap}"; then
>  		echo >&2 Inconsistent kallsyms data
>  		echo >&2 'Try "make KALLSYMS_EXTRA_PASS=1" as a workaround'
> -- 
> 2.34.1
> 

-- 
Nicolas

Re: [RFC PATCH 09/11] kbuild: simplify commands in --dry-run mode

[Nicolas Schier]

On Mon, Aug 19, 2024 at 06:03:06PM +0200, Vegard Nossum wrote:
> - $filechk is used to check if a file is up to date.
> 
> - $cmd includes logic for echoing commands and deleting intermediate
>   files on interrupt. Skip all of that in --dry-run mode and just execute
>   the command.
> 
> - $cmd_and_savecmd executes the command and echoes it into .<target>.cmd.
> 
> - $if_changed_dep executes the command if any dependencies have changed.
> 
> - $cmd_and_fixdep executes the command and updates .<target>.cmd.
> 
> Skip all of that in --dry-run mode and just execute the command.

Why do you want to skip these checks?  The more mechanisms we change for
--dry-run, the less the output shell script is a good way to review with
regard to security issues.  And it "fools" those who want to know what
really happens if 'make' is run w/o --dry-run.

> 
> Signed-off-by: Vegard Nossum <vegard.nossum@oracle.com>
> ---
>  scripts/Kbuild.include | 27 +++++++++++++++++++++++++++
>  1 file changed, 27 insertions(+)
> 
> diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include
> index ed8a7493524b2..a1ef3b1828bb3 100644
> --- a/scripts/Kbuild.include
> +++ b/scripts/Kbuild.include
> @@ -94,6 +94,7 @@ kecho := $($(quiet)kecho)
>  # - If no file exist it is created
>  # - If the content differ the new file is used
>  # - If they are equal no change, and no timestamp update
> +ifndef dry_run
>  define filechk
>  	$(check-FORCE)
>  	$(Q)set -e;						\
> @@ -105,6 +106,14 @@ define filechk
>  		mv -f $(tmp-target) $@;				\
>  	fi
>  endef
> +else
> +# simplify and write the output directly if we're just printing
> +# the commands
> +define filechk
> +	mkdir -p $(dir $@)
> +	{ $(filechk_$(1)); } > $@

To prevent having incomplete or broken output files of filechk, I'd like
to have at least the 'trap' here, too.  E.g.:

	(						\
		set -e;					\
		mkdir -p $(dir $@);			\
		trap "rm -f $@" EXIT;			\
		{ $(filechk_$(1)); } >$@;		\
     	)

Did you see problems with the original filechk implementation?

Kind regards,
Nicolas

Re: [RFC PATCH 10/11] kbuild: don't test for file presence in --dry-run mode

[Nicolas Schier]

On Mon, Aug 19, 2024 at 06:03:07PM +0200, Vegard Nossum wrote:
> I'm not really sure if this is correct as I'm not sure under which
> circumstances the files tested for with $(wildcard) would NOT be
> present. I'm not even sure if this is the right approach to take.
> 
> However, it _should_ keep working the same as before for regular
> 'make' invocations and is necessary for 'make --dry-run' to work.

This might become interesting.  Makefile.modpost is called in several
different situations (e.g. in-tree kernel build, single target builds,
out-of-tree module builds), and if we want to get full shell script
output with --dry-run for all these situations, this probably has to be
refactored in more depth.

Kind regards,
Nicolas

Re: [RFC PATCH 11/11] kbuild: suppress echoing of commands in --dry-run mode

[Nicolas Schier]

On Mon, Aug 19, 2024 at 06:03:08PM +0200, Vegard Nossum wrote:
> If the user ran 'make -n' then we will already print all commands.
> 
> Signed-off-by: Vegard Nossum <vegard.nossum@oracle.com>
> ---
>  Makefile | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index d08ade5791c2e..a1a3e96a10ea2 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -96,9 +96,10 @@ ifneq ($(findstring 1, $(KBUILD_VERBOSE)),)
>    Q =
>  endif
>  
> -# If the user is running make -s (silent mode), suppress echoing of
> -# commands
> -ifneq ($(findstring s,$(firstword -$(MAKEFLAGS))),)
> +# If the user is running make -s (silent mode) or -n (dry run mode),
> +# suppress echoing of commands
> +ifneq (,$(or $(findstring s,$(firstword -$(MAKEFLAGS))), \
> +	$(findstring n,$(firstword -$(MAKEFLAGS)))))
>  quiet=silent_
>  override KBUILD_VERBOSE :=
>  endif
> -- 
> 2.34.1
> 

I think it makes sense to apply this patch earlier.

Reviewed-by: Nicolas Schier <nicolas@fjasle.eu>