All of lore.kernel.org
 help / color / mirror / Atom feed
From: Masahiro Yamada <masahiroy@kernel.org>
To: linux-kbuild@vger.kernel.org
Cc: Michal Simek <michal.simek@xilinx.com>,
	linux-arch@vger.kernel.org, linux-kernel@vger.kernel.org,
	David Howells <dhowells@redhat.com>,
	David Woodhouse <dwmw2@infradead.org>,
	keyrings@vger.kernel.org, Richard Weinberger <richard@nod.at>,
	Masahiro Yamada <masahiroy@kernel.org>
Subject: [PATCH 08/10] kbuild: do not include include/config/auto.conf from shell scripts
Date: Mon, 13 Dec 2021 04:29:39 +0900	[thread overview]
Message-ID: <20211212192941.1149247-9-masahiroy@kernel.org> (raw)
In-Reply-To: <20211212192941.1149247-1-masahiroy@kernel.org>

Richard Weinberger pointed out the risk of sourcing the kernel config
from shell scripts [1], and proposed some patches [2], [3]. It is a good
point, but it took a long time because I was wondering how to fix this.

This commit goes with simple grep approach because there are only a few
scripts including the kernel configuration.

scripts/link_vmlinux.sh has references to a bunch of CONFIG options,
all of which are boolean. I added is_enabled() helper as
scripts/package/{mkdebian,builddeb} do.

scripts/gen_autoksyms.sh uses 'eval', stating "to expand the whitelist
path". I removed it since it is the issue we are trying to fix.

I was a bit worried about the cost of invoking the grep command over
again. I extracted the grep parts from it, and measured the cost. It
was approximately 0.03 sec, which I hope is acceptable.

[test code]

  $ cat test-grep.sh
  #!/bin/sh

  is_enabled() {
          grep -q "^$1=y" include/config/auto.conf
  }

  is_enabled CONFIG_LTO_CLANG
  is_enabled CONFIG_LTO_CLANG
  is_enabled CONFIG_STACK_VALIDATION
  is_enabled CONFIG_UNWINDER_ORC
  is_enabled CONFIG_FTRACE_MCOUNT_USE_OBJTOOL
  is_enabled CONFIG_VMLINUX_VALIDATION
  is_enabled CONFIG_FRAME_POINTER
  is_enabled CONFIG_GCOV_KERNEL
  is_enabled CONFIG_LTO_CLANG
  is_enabled CONFIG_RETPOLINE
  is_enabled CONFIG_X86_SMAP
  is_enabled CONFIG_LTO_CLANG
  is_enabled CONFIG_VMLINUX_MAP
  is_enabled CONFIG_KALLSYMS_ALL
  is_enabled CONFIG_KALLSYMS_ABSOLUTE_PERCPU
  is_enabled CONFIG_KALLSYMS_BASE_RELATIVE
  is_enabled CONFIG_DEBUG_INFO_BTF
  is_enabled CONFIG_KALLSYMS
  is_enabled CONFIG_DEBUG_INFO_BTF
  is_enabled CONFIG_BPF
  is_enabled CONFIG_BUILDTIME_TABLE_SORT
  is_enabled CONFIG_KALLSYMS

  $ time ./test-grep.sh
  real    0m0.036s
  user    0m0.027s
  sys     m0.009s

[1]: https://lore.kernel.org/all/1919455.eZKeABUfgV@blindfold/
[2]: https://lore.kernel.org/all/20180219092245.26404-1-richard@nod.at/
[3]: https://lore.kernel.org/all/20210920213957.1064-2-richard@nod.at/
Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

 scripts/gen_autoksyms.sh |  6 ++---
 scripts/link-vmlinux.sh  | 47 ++++++++++++++++++++--------------------
 scripts/setlocalversion  |  9 ++++----
 3 files changed, 30 insertions(+), 32 deletions(-)

diff --git a/scripts/gen_autoksyms.sh b/scripts/gen_autoksyms.sh
index 6ed0d225c8b1..12ffb01f13cb 100755
--- a/scripts/gen_autoksyms.sh
+++ b/scripts/gen_autoksyms.sh
@@ -26,10 +26,8 @@ if [ -n "$CONFIG_MODVERSIONS" ]; then
 	needed_symbols="$needed_symbols module_layout"
 fi
 
-ksym_wl=
-if [ -n "$CONFIG_UNUSED_KSYMS_WHITELIST" ]; then
-	# Use 'eval' to expand the whitelist path and check if it is relative
-	eval ksym_wl="$CONFIG_UNUSED_KSYMS_WHITELIST"
+ksym_wl=$(sed -n 's/^CONFIG_UNUSED_KSYMS_WHITELIST="\(.*\)"$/\1/p' include/config/auto.conf)
+if [ -n "$ksym_wl" ]; then
 	[ "${ksym_wl}" != "${ksym_wl#/}" ] || ksym_wl="$abs_srctree/$ksym_wl"
 	if [ ! -f "$ksym_wl" ] || [ ! -r "$ksym_wl" ]; then
 		echo "ERROR: '$ksym_wl' whitelist file not found" >&2
diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
index 5cdd9bc5c385..a4b61a2f65db 100755
--- a/scripts/link-vmlinux.sh
+++ b/scripts/link-vmlinux.sh
@@ -34,6 +34,10 @@ LD="$1"
 KBUILD_LDFLAGS="$2"
 LDFLAGS_vmlinux="$3"
 
+is_enabled() {
+	grep -q "^$1=y" include/config/auto.conf
+}
+
 # Nice output in kbuild format
 # Will be supressed by "make -s"
 info()
@@ -80,11 +84,11 @@ modpost_link()
 		${KBUILD_VMLINUX_LIBS}				\
 		--end-group"
 
-	if [ -n "${CONFIG_LTO_CLANG}" ]; then
+	if is_enabled CONFIG_LTO_CLANG; then
 		gen_initcalls
 		lds="-T .tmp_initcalls.lds"
 
-		if [ -n "${CONFIG_MODVERSIONS}" ]; then
+		if is_enabled CONFIG_MODVERSIONS; then
 			gen_symversions
 			lds="${lds} -T .tmp_symversions.lds"
 		fi
@@ -104,21 +108,21 @@ objtool_link()
 	local objtoolcmd;
 	local objtoolopt;
 
-	if [ "${CONFIG_LTO_CLANG} ${CONFIG_STACK_VALIDATION}" = "y y" ]; then
+	if is_enabled CONFIG_LTO_CLANG && is_enabled CONFIG_STACK_VALIDATION; then
 		# Don't perform vmlinux validation unless explicitly requested,
 		# but run objtool on vmlinux.o now that we have an object file.
-		if [ -n "${CONFIG_UNWINDER_ORC}" ]; then
+		if is_enabled CONFIG_UNWINDER_ORC; then
 			objtoolcmd="orc generate"
 		fi
 
 		objtoolopt="${objtoolopt} --duplicate"
 
-		if [ -n "${CONFIG_FTRACE_MCOUNT_USE_OBJTOOL}" ]; then
+		if is_enabled CONFIG_FTRACE_MCOUNT_USE_OBJTOOL; then
 			objtoolopt="${objtoolopt} --mcount"
 		fi
 	fi
 
-	if [ -n "${CONFIG_VMLINUX_VALIDATION}" ]; then
+	if is_enabled CONFIG_VMLINUX_VALIDATION; then
 		objtoolopt="${objtoolopt} --noinstr"
 	fi
 
@@ -127,16 +131,16 @@ objtool_link()
 			objtoolcmd="check"
 		fi
 		objtoolopt="${objtoolopt} --vmlinux"
-		if [ -z "${CONFIG_FRAME_POINTER}" ]; then
+		if ! is_enabled CONFIG_FRAME_POINTER; then
 			objtoolopt="${objtoolopt} --no-fp"
 		fi
-		if [ -n "${CONFIG_GCOV_KERNEL}" ] || [ -n "${CONFIG_LTO_CLANG}" ]; then
+		if is_enabled CONFIG_GCOV_KERNEL || is_enabled CONFIG_LTO_CLANG; then
 			objtoolopt="${objtoolopt} --no-unreachable"
 		fi
-		if [ -n "${CONFIG_RETPOLINE}" ]; then
+		if is_enabled CONFIG_RETPOLINE; then
 			objtoolopt="${objtoolopt} --retpoline"
 		fi
-		if [ -n "${CONFIG_X86_SMAP}" ]; then
+		if is_enabled CONFIG_X86_SMAP; then
 			objtoolopt="${objtoolopt} --uaccess"
 		fi
 		info OBJTOOL ${1}
@@ -161,7 +165,7 @@ vmlinux_link()
 	# skip output file argument
 	shift
 
-	if [ -n "${CONFIG_LTO_CLANG}" ]; then
+	if is_enabled CONFIG_LTO_CLANG; then
 		# Use vmlinux.o instead of performing the slow LTO link again.
 		objs=vmlinux.o
 		libs=
@@ -189,7 +193,7 @@ vmlinux_link()
 		ldflags="${ldflags} ${wl}--strip-debug"
 	fi
 
-	if [ -n "${CONFIG_VMLINUX_MAP}" ]; then
+	if is_enabled CONFIG_VMLINUX_MAP; then
 		ldflags="${ldflags} ${wl}-Map=${output}.map"
 	fi
 
@@ -239,15 +243,15 @@ kallsyms()
 {
 	local kallsymopt;
 
-	if [ -n "${CONFIG_KALLSYMS_ALL}" ]; then
+	if is_enabled CONFIG_KALLSYMS_ALL; then
 		kallsymopt="${kallsymopt} --all-symbols"
 	fi
 
-	if [ -n "${CONFIG_KALLSYMS_ABSOLUTE_PERCPU}" ]; then
+	if is_enabled CONFIG_KALLSYMS_ABSOLUTE_PERCPU; then
 		kallsymopt="${kallsymopt} --absolute-percpu"
 	fi
 
-	if [ -n "${CONFIG_KALLSYMS_BASE_RELATIVE}" ]; then
+	if is_enabled CONFIG_KALLSYMS_BASE_RELATIVE; then
 		kallsymopt="${kallsymopt} --base-relative"
 	fi
 
@@ -312,9 +316,6 @@ if [ "$1" = "clean" ]; then
 	exit 0
 fi
 
-# We need access to CONFIG_ symbols
-. include/config/auto.conf
-
 # Update version
 info GEN .version
 if [ -r .version ]; then
@@ -343,7 +344,7 @@ tr '\0' '\n' < modules.builtin.modinfo | sed -n 's/^[[:alnum:]:_]*\.file=//p' |
 	tr ' ' '\n' | uniq | sed -e 's:^:kernel/:' -e 's/$/.ko/' > modules.builtin
 
 btf_vmlinux_bin_o=""
-if [ -n "${CONFIG_DEBUG_INFO_BTF}" ]; then
+if is_enabled CONFIG_DEBUG_INFO_BTF; then
 	btf_vmlinux_bin_o=.btf.vmlinux.bin.o
 	if ! gen_btf .tmp_vmlinux.btf $btf_vmlinux_bin_o ; then
 		echo >&2 "Failed to generate BTF for vmlinux"
@@ -355,7 +356,7 @@ fi
 kallsymso=""
 kallsymso_prev=""
 kallsyms_vmlinux=""
-if [ -n "${CONFIG_KALLSYMS}" ]; then
+if is_enabled CONFIG_KALLSYMS; then
 
 	# kallsyms support
 	# Generate section listing all symbols and add it into vmlinux
@@ -395,12 +396,12 @@ fi
 vmlinux_link vmlinux "${kallsymso}" ${btf_vmlinux_bin_o}
 
 # fill in BTF IDs
-if [ -n "${CONFIG_DEBUG_INFO_BTF}" -a -n "${CONFIG_BPF}" ]; then
+if is_enabled CONFIG_DEBUG_INFO_BTF && is_enabled CONFIG_BPF; then
 	info BTFIDS vmlinux
 	${RESOLVE_BTFIDS} vmlinux
 fi
 
-if [ -n "${CONFIG_BUILDTIME_TABLE_SORT}" ]; then
+if is_enabled CONFIG_BUILDTIME_TABLE_SORT; then
 	info SORTTAB vmlinux
 	if ! sorttable vmlinux; then
 		echo >&2 Failed to sort kernel tables
@@ -412,7 +413,7 @@ info SYSMAP System.map
 mksysmap vmlinux System.map
 
 # step a (see comment above)
-if [ -n "${CONFIG_KALLSYMS}" ]; then
+if is_enabled CONFIG_KALLSYMS; then
 	mksysmap ${kallsyms_vmlinux} .tmp_System.map
 
 	if ! cmp -s System.map .tmp_System.map; then
diff --git a/scripts/setlocalversion b/scripts/setlocalversion
index 6b54e46a0f12..d06137405190 100755
--- a/scripts/setlocalversion
+++ b/scripts/setlocalversion
@@ -111,9 +111,7 @@ if $scm_only; then
 	exit
 fi
 
-if test -e include/config/auto.conf; then
-	. include/config/auto.conf
-else
+if ! test -e include/config/auto.conf; then
 	echo "Error: kernelrelease not valid - run 'make prepare' to update it" >&2
 	exit 1
 fi
@@ -125,10 +123,11 @@ if test ! "$srctree" -ef .; then
 fi
 
 # CONFIG_LOCALVERSION and LOCALVERSION (if set)
-res="${res}${CONFIG_LOCALVERSION}${LOCALVERSION}"
+config_localversion=$(sed -n 's/^CONFIG_LOCALVERSION="\(.*\)"$/\1/p' include/config/auto.conf)
+res="${res}${config_localversion}${LOCALVERSION}"
 
 # scm version string if not at a tagged commit
-if test "$CONFIG_LOCALVERSION_AUTO" = "y"; then
+if grep -q "^CONFIG_LOCALVERSION_AUTO=y$" include/config/auto.conf; then
 	# full scm version string
 	res="$res$(scm_version)"
 elif [ "${LOCALVERSION+set}" != "set" ]; then
-- 
2.32.0


  parent reply	other threads:[~2021-12-12 19:31 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-12-12 19:29 [PATCH 00/10] kbuild: do not quote string values in Makefile Masahiro Yamada
2021-12-12 19:29 ` [PATCH 01/10] certs: use $@ to simplify the key generation rule Masahiro Yamada
2021-12-13 13:00   ` Nicolas Schier
2021-12-14  1:45     ` Masahiro Yamada
2021-12-12 19:29 ` [PATCH 02/10] certs: unify duplicated cmd_extract_certs and improve the log Masahiro Yamada
2021-12-13 13:07   ` Nicolas Schier
2021-12-12 19:29 ` [PATCH 03/10] certs: remove unneeded -I$(srctree) option for system_certificates.o Masahiro Yamada
2021-12-12 19:29 ` [PATCH 04/10] certs: refactor file cleaning Masahiro Yamada
2021-12-13 13:08   ` Nicolas Schier
2021-12-12 19:29 ` [PATCH 05/10] certs: remove misleading comments about GCC PR Masahiro Yamada
2021-12-12 19:29 ` [PATCH 06/10] kbuild: stop using config_filename in scripts/Makefile.modsign Masahiro Yamada
2021-12-13 13:13   ` Nicolas Schier
2021-12-12 19:29 ` [PATCH 07/10] certs: simplify $(srctree)/ handling and remove config_filename macro Masahiro Yamada
2021-12-12 19:29 ` Masahiro Yamada [this message]
2021-12-13 13:17   ` [PATCH 08/10] kbuild: do not include include/config/auto.conf from shell scripts Nicolas Schier
2021-12-14  1:50     ` Masahiro Yamada
2021-12-12 19:29 ` [PATCH 09/10] kbuild: do not quote string values in include/config/auto.conf Masahiro Yamada
2021-12-12 19:29 ` [PATCH 10/10] microblaze: use built-in function to get CPU_{MAJOR,MINOR,REV} Masahiro Yamada
2021-12-13 13:18   ` Nicolas Schier

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20211212192941.1149247-9-masahiroy@kernel.org \
    --to=masahiroy@kernel.org \
    --cc=dhowells@redhat.com \
    --cc=dwmw2@infradead.org \
    --cc=keyrings@vger.kernel.org \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-kbuild@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=michal.simek@xilinx.com \
    --cc=richard@nod.at \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.