linux-kernel - [PATCH v2] kbuild: pahole-version: improve overall checking and error messages

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-next>] [day] [month] [year] [list]

Message-ID: <20240902160828.1092891-1-ojeda@kernel.org>
Date: Mon,  2 Sep 2024 18:08:28 +0200
From: Miguel Ojeda <ojeda@...nel.org>
To: Masahiro Yamada <masahiroy@...nel.org>
Cc: Nathan Chancellor <nathan@...nel.org>,
	Nicolas Schier <nicolas@...sle.eu>,
	linux-kbuild@...r.kernel.org,
	linux-kernel@...r.kernel.org,
	patches@...ts.linux.dev,
	Miguel Ojeda <ojeda@...nel.org>
Subject: [PATCH v2] kbuild: pahole-version: improve overall checking and error messages

Like patch "rust: suppress error messages from
CONFIG_{RUSTC,BINDGEN}_VERSION_TEXT" [1], do not assume the file existing
and being executable implies executing it will succeed.

Instead, bail out if executing it fails for any reason, as well as if
the program does not print to standard output what we are expecting from
`pahole --version`. The script needs to ensure that it always returns
an integer, since its output will go into a Kconfig `int`-type symbol.

In addition, check whether the program exists first, and provide
error messages for each error condition, similar to how it is done in
e.g. `scripts/rust_is_available.sh`.

For instance, currently `pahole` may be built for another architecture
or may be a program we do not expect that fails:

    $ echo 'bad' > bad-pahole
    $ chmod u+x bad-pahole
    $ make PAHOLE=./bad-pahole defconfig
    ...
    ./bad-pahole: 1: bad: not found
    init/Kconfig:112: syntax error
    init/Kconfig:112: invalid statement

With this applied, we get instead:

    ***
    *** Running './bad-pahole' to check the pahole version failed with
    *** code 127. pahole will not be used.
    ***
    ...
    $ grep CONFIG_PAHOLE_VERSION .config
    CONFIG_PAHOLE_VERSION=0

Similarly, `pahole` currently may be a program that returns successfully,
but that does not print the version that we would expect:

    $ echo 'echo' > bad-pahole
    $ chmod u+x bad-pahole
    $ make PAHOLE=./bad-pahole defconfig
    ...
    init/Kconfig:114: syntax error
    init/Kconfig:114: invalid statement

With this applied, we get instead:

    ***
    *** pahole './bad-pahole' returned an unexpected version output.
    *** pahole will not be used.
    ***
    ...
    $ grep CONFIG_PAHOLE_VERSION .config
    CONFIG_PAHOLE_VERSION=0

Finally, with this applied, if the program does not exist, we get:

    $ make PAHOLE=./bad-pahole defconfig
    ...
    ***
    *** pahole './bad-pahole' could not be found. pahole will not be used.
    ***
    ...
    $ grep CONFIG_PAHOLE_VERSION .config
    CONFIG_PAHOLE_VERSION=0

Link: https://lore.kernel.org/rust-for-linux/20240727140302.1806011-1-masahiroy@kernel.org/ [1]
Co-developed-by: Nicolas Schier <nicolas@...sle.eu>
Signed-off-by: Nicolas Schier <nicolas@...sle.eu>
Signed-off-by: Miguel Ojeda <ojeda@...nel.org>
---
v1: https://lore.kernel.org/all/20240728125527.690726-1-ojeda@kernel.org/
v2:

Reworked to catch successful programs that may not output what we expect from
pahole as well as to do the checking step-by-step (for better error messages).

I didn't change the regular expression to reduce the changes (except adding
`p`) -- it can be improved separately.

Also, please note that I added Nicolas as co-author since he proposed part of
the solution, but he has not formally signed off yet.

 scripts/pahole-version.sh | 30 +++++++++++++++++++++++++++---
 1 file changed, 27 insertions(+), 3 deletions(-)

diff --git a/scripts/pahole-version.sh b/scripts/pahole-version.sh
index f8a32ab93ad1..cdb80a3d6e4f 100755
--- a/scripts/pahole-version.sh
+++ b/scripts/pahole-version.sh
@@ -5,9 +5,33 @@
 #
 # Prints pahole's version in a 3-digit form, such as 119 for v1.19.

-if [ ! -x "$(command -v "$@")" ]; then
-	echo 0
+set -e
+trap "echo 0; exit 1" EXIT
+
+if ! command -v "$@" >/dev/null; then
+	echo >&2 "***"
+	echo >&2 "*** pahole '$@' could not be found. pahole will not be used."
+	echo >&2 "***"
+	exit 1
+fi
+
+output=$("$@" --version 2>/dev/null) || code=$?
+if [ -n "$code" ]; then
+	echo >&2 "***"
+	echo >&2 "*** Running '$@' to check the pahole version failed with"
+	echo >&2 "*** code $code. pahole will not be used."
+	echo >&2 "***"
+	exit 1
+fi
+
+output=$(echo "$output" | sed -nE 's/v([0-9]+)\.([0-9]+)/\1\2/p')
+if [ -z "${output}" ]; then
+	echo >&2 "***"
+	echo >&2 "*** pahole '$@' returned an unexpected version output."
+	echo >&2 "*** pahole will not be used."
+	echo >&2 "***"
 	exit 1
 fi

-"$@" --version | sed -E 's/v([0-9]+)\.([0-9]+)/\1\2/'
+echo "${output}"
+trap EXIT

base-commit: 431c1646e1f86b949fa3685efc50b660a364c2b6
--
2.46.0