Skip to content

Commit

Permalink
fortify: refactor test_fortify Makefile to fix some build problems
Browse files Browse the repository at this point in the history
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-<PID>' 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 <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Kees Cook <[email protected]>
  • Loading branch information
masahir0y authored and kees committed Aug 15, 2024
1 parent de9c2c6 commit 4e9903b
Show file tree
Hide file tree
Showing 5 changed files with 33 additions and 39 deletions.
2 changes: 0 additions & 2 deletions lib/.gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,3 @@
/gen_crc32table
/gen_crc64table
/oid_registry_data.c
/test_fortify.log
/test_fortify/*.log
38 changes: 1 addition & 37 deletions lib/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -393,40 +393,4 @@ obj-$(CONFIG_GENERIC_LIB_DEVMEM_IS_ALLOWED) += devmem_is_allowed.o

obj-$(CONFIG_FIRMWARE_TABLE) += fw_table.o

# FORTIFY_SOURCE compile-time behavior tests
TEST_FORTIFY_SRCS = $(wildcard $(src)/test_fortify/*-*.c)
TEST_FORTIFY_LOGS = $(patsubst $(src)/%.c, %.log, $(TEST_FORTIFY_SRCS))
TEST_FORTIFY_LOG = test_fortify.log

quiet_cmd_test_fortify = TEST $@
cmd_test_fortify = $(CONFIG_SHELL) $(srctree)/scripts/test_fortify.sh \
$< $@ "$(NM)" $(CC) $(c_flags) \
$(call cc-disable-warning,fortify-source) \
-DKBUILD_EXTRA_WARN1

targets += $(TEST_FORTIFY_LOGS)
clean-files += $(TEST_FORTIFY_LOGS)
clean-files += $(addsuffix .o, $(TEST_FORTIFY_LOGS))
$(obj)/test_fortify/%.log: $(src)/test_fortify/%.c \
$(src)/test_fortify/test_fortify.h \
$(srctree)/include/linux/fortify-string.h \
$(srctree)/scripts/test_fortify.sh \
FORCE
$(call if_changed,test_fortify)

quiet_cmd_gen_fortify_log = GEN $@
cmd_gen_fortify_log = cat </dev/null $(filter-out FORCE,$^) 2>/dev/null > $@ || true

targets += $(TEST_FORTIFY_LOG)
clean-files += $(TEST_FORTIFY_LOG)
$(obj)/$(TEST_FORTIFY_LOG): $(addprefix $(obj)/, $(TEST_FORTIFY_LOGS)) FORCE
$(call if_changed,gen_fortify_log)

# Fake dependency to trigger the fortify tests.
ifeq ($(CONFIG_FORTIFY_SOURCE),y)
$(obj)/string.o: $(obj)/$(TEST_FORTIFY_LOG)
endif

# Some architectures define __NO_FORTIFY if __SANITIZE_ADDRESS__ is undefined.
# Pass CFLAGS_KASAN to avoid warnings.
$(foreach x, $(patsubst %.log,%.o,$(TEST_FORTIFY_LOGS)), $(eval KASAN_SANITIZE_$(x) := y))
subdir-$(CONFIG_FORTIFY_SOURCE) += test_fortify
2 changes: 2 additions & 0 deletions lib/test_fortify/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
# SPDX-License-Identifier: GPL-2.0-only
/*.log
28 changes: 28 additions & 0 deletions lib/test_fortify/Makefile
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
# SPDX-License-Identifier: GPL-2.0

ccflags-y := $(call cc-disable-warning,fortify-source)

quiet_cmd_test_fortify = TEST $@
cmd_test_fortify = $(CONFIG_SHELL) $(srctree)/scripts/test_fortify.sh \
$< $@ "$(NM)" $(CC) $(c_flags) -DKBUILD_EXTRA_WARN1

$(obj)/%.log: $(src)/%.c $(srctree)/scripts/test_fortify.sh \
$(src)/test_fortify.h \
$(srctree)/include/linux/fortify-string.h \
FORCE
$(call if_changed,test_fortify)

logs = $(patsubst $(src)/%.c, %.log, $(wildcard $(src)/*-*.c))
targets += $(logs)

quiet_cmd_gen_fortify_log = CAT $@
cmd_gen_fortify_log = cat $(or $(real-prereqs),/dev/null) > $@

$(obj)/test_fortify.log: $(addprefix $(obj)/, $(logs)) FORCE
$(call if_changed,gen_fortify_log)

always-y += test_fortify.log

# Some architectures define __NO_FORTIFY if __SANITIZE_ADDRESS__ is undefined.
# Pass CFLAGS_KASAN to avoid warnings.
KASAN_SANITIZE := y
2 changes: 2 additions & 0 deletions scripts/remove-stale-files
Original file line number Diff line number Diff line change
Expand Up @@ -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

0 comments on commit 4e9903b

Please sign in to comment.