Re: [PATCH v2 1/2] selftests/livepatch: build test modules against the selected kernel

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] selftests/livepatch: clean fails to resolve LIVEPATCH_OBJTREE
- [Medium] selftests/livepatch: relative command-line KDIR not resolved for sub-make
--

commit 02a5cf889b06dba8630406e570f3de0adc2db5d6
Author: Sasha Levin <sashal@kernel.org>

selftests/livepatch: build test modules against the selected kernel

This commit updates the livepatch selftest Makefiles to select KDIR from O,
KBUILD_OUTPUT, or the in-tree build before falling back to the running
kernel's build directory. It also skips building test modules when
CONFIG_LIVEPATCH is disabled in the target .config and stops forwarding O and
KBUILD_OUTPUT to the external module build.

> diff --git a/tools/testing/selftests/livepatch/Makefile b/tools/testing/selftests/livepatch/Makefile
> index a080eb54a215d..855bbad241235 100644
> --- a/tools/testing/selftests/livepatch/Makefile
> +++ b/tools/testing/selftests/livepatch/Makefile
> @@ -1,7 +1,25 @@
>  # SPDX-License-Identifier: GPL-20
>  
> +ifndef KDIR
> +LIVEPATCH_OBJTREE := $(firstword $(O) $(KBUILD_OUTPUT))
> +ifneq ($(and $(LIVEPATCH_OBJTREE),$(wildcard $(LIVEPATCH_OBJTREE)/Module.symvers)),)
> +KDIR := $(LIVEPATCH_OBJTREE)
> +else ifneq ($(wildcard ../../../../Module.symvers),)
> +KDIR := ../../../..
> +else
> +KDIR := /lib/modules/$(shell uname -r)/build
> +endif
> +endif

[Severity: Medium]
Can this fail to locate the build directory during a clean when a relative
O= or KBUILD_OUTPUT= path is used, or when the kernel build tree has already
removed Module.symvers?

During all, run_tests, and install, tools/testing/selftests/Makefile resolves
O and KBUILD_OUTPUT into abs_objtree and passes O=$(abs_objtree) to the
target sub-makes, but the clean target does not pass O=$(abs_objtree):

tools/testing/selftests/Makefile:
clean:
	@for TARGET in $(TARGETS) $(INSTALL_DEP_TARGETS); do \
		BUILD_TARGET=$$BUILD/$$TARGET;	\
		$(MAKE) OUTPUT=$$BUILD_TARGET -C $$TARGET clean;\
	done;

When running "make O=build kselftest-clean TARGETS=livepatch" or
"make -C tools/testing/selftests O=build TARGETS=livepatch clean",
LIVEPATCH_OBJTREE in tools/testing/selftests/livepatch/Makefile receives the
unanchored relative path "build" (via MAKEFLAGS or the KBUILD_OUTPUT
environment variable) and checks for tools/testing/selftests/livepatch/build/Module.symvers
instead of ../../../../build/Module.symvers.

Similarly, if the kernel build directory was cleaned before kselftest-clean,
Module.symvers is already gone even though Kbuild external-module clean does
not require it.

In both cases, KDIR falls back to /lib/modules/$(shell uname -r)/build. On a
host or container where /lib/modules/$(uname -r)/build does not exist,
tools/testing/selftests/livepatch/test_modules/Makefile sees an empty
$(wildcard $(KDIR)) and skips cleaning test_modules/, leaving the built
module artifacts behind.

> +override KDIR := $(abspath $(KDIR))
> +export KDIR

[Severity: Medium]
Does passing a relative KDIR=... on the make command line still propagate the
unresolved relative path to test_modules/Makefile?

In GNU Make, "override KDIR := $(abspath $(KDIR))" and "export KDIR" update
KDIR in the current makefile and exported environment, but the original
command-line assignment remains in MAKEOVERRIDES (forwarded via MAKEFLAGS).

When ../lib.mk (via gen_mods_dir or clean_mods_dir) or clean_livepatch_mods
invokes $(MAKE) -C test_modules, the sub-make in
tools/testing/selftests/livepatch/test_modules/Makefile only filters O=% and
KBUILD_OUTPUT=% out of MAKEOVERRIDES:

tools/testing/selftests/livepatch/test_modules/Makefile:
modules clean: MAKEOVERRIDES := $(filter-out O=% KBUILD_OUTPUT=%,$(MAKEOVERRIDES))

As a result, the child make inherits the raw command-line KDIR=<relative_path>
from MAKEFLAGS, which takes precedence over the exported environment variable
and is evaluated one directory deeper inside test_modules/ (for example,
"make -C tools/testing/selftests/livepatch KDIR=../../../.." resolves KDIR to
tools/ inside test_modules/Makefile and fails the build, or silently skips
building if the shifted path does not exist).

Should KDIR=% also be filtered out of MAKEOVERRIDES (or passed explicitly as
KDIR=$(KDIR) to the test_modules sub-make)?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261103140218.17568-1-sashal@kernel.org?part=1