From 4e9903b0861c9df3464b82db4a7025863bac1897 Mon Sep 17 00:00:00 2001 From: Masahiro Yamada Date: Sun, 28 Jul 2024 00:02:36 +0900 Subject: fortify: refactor test_fortify Makefile to fix some build problems There are some issues in the test_fortify Makefile code. Problem 1: cc-disable-warning invokes compiler dozens of times To see how many times the cc-disable-warning is evaluated, change this code: $(call cc-disable-warning,fortify-source) to: $(call cc-disable-warning,$(shell touch /tmp/fortify-$$$$)fortify-source) Then, build the kernel with CONFIG_FORTIFY_SOURCE=y. You will see a large number of '/tmp/fortify-' files created: $ ls -1 /tmp/fortify-* | wc 80 80 1600 This means the compiler was invoked 80 times just for checking the -Wno-fortify-source flag support. $(call cc-disable-warning,fortify-source) should be added to a simple variable instead of a recursive variable. Problem 2: do not recompile string.o when the test code is updated The test cases are independent of the kernel. However, when the test code is updated, $(obj)/string.o is rebuilt and vmlinux is relinked due to this dependency: $(obj)/string.o: $(obj)/$(TEST_FORTIFY_LOG) always-y is suitable for building the log files. Problem 3: redundant code clean-files += $(addsuffix .o, $(TEST_FORTIFY_LOGS)) ... is unneeded because the top Makefile globally cleans *.o files. This commit fixes these issues and makes the code readable. Signed-off-by: Masahiro Yamada Link: https://lore.kernel.org/r/20240727150302.1823750-2-masahiroy@kernel.org Signed-off-by: Kees Cook --- scripts/remove-stale-files | 2 ++ 1 file changed, 2 insertions(+) (limited to 'scripts') diff --git a/scripts/remove-stale-files b/scripts/remove-stale-files index f38d26b78c2a..8fc55a749ccc 100755 --- a/scripts/remove-stale-files +++ b/scripts/remove-stale-files @@ -21,3 +21,5 @@ set -e # then will be really dead and removed from the code base entirely. rm -f *.spec + +rm -f lib/test_fortify.log -- cgit v1.2.3 From 5a8d0c46c9e024bed4805a9335fe6124d8a78d3a Mon Sep 17 00:00:00 2001 From: Masahiro Yamada Date: Sun, 28 Jul 2024 00:02:37 +0900 Subject: fortify: move test_fortify.sh to lib/test_fortify/ This script is only used in lib/test_fortify/. There is no reason to keep it in scripts/. Signed-off-by: Masahiro Yamada Link: https://lore.kernel.org/r/20240727150302.1823750-3-masahiroy@kernel.org Signed-off-by: Kees Cook --- scripts/test_fortify.sh | 66 ------------------------------------------------- 1 file changed, 66 deletions(-) delete mode 100644 scripts/test_fortify.sh (limited to 'scripts') diff --git a/scripts/test_fortify.sh b/scripts/test_fortify.sh deleted file mode 100644 index c2688ab8281d..000000000000 --- a/scripts/test_fortify.sh +++ /dev/null @@ -1,66 +0,0 @@ -#!/bin/sh -# SPDX-License-Identifier: GPL-2.0-only -set -e - -# Argument 1: Source file to build. -IN="$1" -shift -# Extract just the filename for error messages below. -FILE="${IN##*/}" -# Extract the function name for error messages below. -FUNC="${FILE#*-}" -FUNC="${FUNC%%-*}" -FUNC="${FUNC%%.*}" -# Extract the symbol to test for in build/symbol test below. -WANT="__${FILE%%-*}" - -# Argument 2: Where to write the build log. -OUT="$1" -shift -TMP="${OUT}.tmp" - -# Argument 3: Path to "nm" tool. -NM="$1" -shift - -# Remaining arguments are: $(CC) $(c_flags) - -# Clean up temporary file at exit. -__cleanup() { - rm -f "$TMP" -} -trap __cleanup EXIT - -# Function names in warnings are wrapped in backticks under UTF-8 locales. -# Run the commands with LANG=C so that grep output will not change. -export LANG=C - -status= -# Attempt to build a source that is expected to fail with a specific warning. -if "$@" -Werror -c "$IN" -o "$OUT".o 2> "$TMP" ; then - # If the build succeeds, either the test has failed or the - # warning may only happen at link time (Clang). In that case, - # make sure the expected symbol is unresolved in the symbol list. - # If so, FORTIFY is working for this case. - if ! $NM -A "$OUT".o | grep -m1 "\bU ${WANT}$" >>"$TMP" ; then - status="warning: unsafe ${FUNC}() usage lacked '$WANT' symbol in $IN" - fi -else - # If the build failed, check for the warning in the stderr. - # GCC: - # ./include/linux/fortify-string.h:316:25: error: call to '__write_overflow_field' declared with attribute warning: detected write beyond size of field (1st parameter); maybe use struct_group()? [-Werror=attribute-warning] - # Clang 14: - # ./include/linux/fortify-string.h:316:4: error: call to __write_overflow_field declared with 'warning' attribute: detected write beyond size of field (1st parameter); maybe use struct_group()? [-Werror,-Wattribute-warning] - if ! grep -Eq -m1 "error: call to .?\b${WANT}\b.?" "$TMP" ; then - status="warning: unsafe ${FUNC}() usage lacked '$WANT' warning in $IN" - fi -fi - -if [ -n "$status" ]; then - # Report on failure results, including compilation warnings. - echo "$status" | tee "$OUT" >&2 -else - # Report on good results, and save any compilation output to log. - echo "ok: unsafe ${FUNC}() usage correctly detected with '$WANT' in $IN" >"$OUT" -fi -cat "$TMP" >>"$OUT" -- cgit v1.2.3 From 9b97452bcce77f8ef29b20c9662d95988b5990e4 Mon Sep 17 00:00:00 2001 From: Michal Wajdeczko Date: Thu, 25 Jul 2024 12:18:41 +0200 Subject: coccinelle: Add rules to find str_up_down() replacements Add rules for finding places where str_up_down() can be used. This currently finds over 20 locations. Signed-off-by: Michal Wajdeczko Link: https://lore.kernel.org/r/20240725101841.574-2-michal.wajdeczko@intel.com Signed-off-by: Kees Cook --- scripts/coccinelle/api/string_choices.cocci | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) (limited to 'scripts') diff --git a/scripts/coccinelle/api/string_choices.cocci b/scripts/coccinelle/api/string_choices.cocci index a71966c0494e..d517f6bc850b 100644 --- a/scripts/coccinelle/api/string_choices.cocci +++ b/scripts/coccinelle/api/string_choices.cocci @@ -39,3 +39,26 @@ e << str_plural_r.E; @@ coccilib.report.print_report(p[0], "opportunity for str_plural(%s)" % e) + +@str_up_down depends on patch@ +expression E; +@@ +( +- ((E) ? "up" : "down") ++ str_up_down(E) +) + +@str_up_down_r depends on !patch exists@ +expression E; +position P; +@@ +( +* ((E@P) ? "up" : "down") +) + +@script:python depends on report@ +p << str_up_down_r.P; +e << str_up_down_r.E; +@@ + +coccilib.report.print_report(p[0], "opportunity for str_up_down(%s)" % e) -- cgit v1.2.3 From 0336f898881ae13b92dfd8b72e69ed1246eac762 Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Mon, 12 Aug 2024 11:36:38 -0700 Subject: coccinelle: Add rules to find str_down_up() replacements As done with str_up_down(), add checks for str_down_up() opportunities. 5 cases currently exist in the tree. Suggested-by: Andy Shevchenko Link: https://lore.kernel.org/r/20240812183637.work.999-kees@kernel.org Reviewed-by: Andy Shevchenko Signed-off-by: Kees Cook --- scripts/coccinelle/api/string_choices.cocci | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) (limited to 'scripts') diff --git a/scripts/coccinelle/api/string_choices.cocci b/scripts/coccinelle/api/string_choices.cocci index d517f6bc850b..5e729f187f22 100644 --- a/scripts/coccinelle/api/string_choices.cocci +++ b/scripts/coccinelle/api/string_choices.cocci @@ -62,3 +62,26 @@ e << str_up_down_r.E; @@ coccilib.report.print_report(p[0], "opportunity for str_up_down(%s)" % e) + +@str_down_up depends on patch@ +expression E; +@@ +( +- ((E) ? "down" : "up") ++ str_down_up(E) +) + +@str_down_up_r depends on !patch exists@ +expression E; +position P; +@@ +( +* ((E@P) ? "down" : "up") +) + +@script:python depends on report@ +p << str_down_up_r.P; +e << str_down_up_r.E; +@@ + +coccilib.report.print_report(p[0], "opportunity for str_down_up(%s)" % e) -- cgit v1.2.3