Re: [PATCH bpf-next v8 02/11] selftests/bpf: Fix test_kmods KDIR to honor O= and distro kernels

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Incorrect Kbuild variable precedence when both O= and KBUILD_OUTPUT= are set with different values.
--

commit 9fabc0968b75e1e4a5267d98394811c6db31a86e
Author: Ricardo B. Marlière <rbm@suse.com>

selftests/bpf: Fix test_kmods KDIR to honor O= and distro kernels

test_kmods/Makefile always pointed KDIR at the kernel source tree root,
ignoring O= and KBUILD_OUTPUT. On distro kernels where the source tree has
not been built, there was no fallback and the build would fail
unconditionally.

> diff --git a/tools/testing/selftests/bpf/test_kmods/Makefile b/tools/testing/selftests/bpf/test_kmods/Makefile
> index 63c4d3f6a12f..dcba595b4b99 100644
> --- a/tools/testing/selftests/bpf/test_kmods/Makefile
> +++ b/tools/testing/selftests/bpf/test_kmods/Makefile

[ ... ]

> @@ -14,8 +25,21 @@ $(foreach m,$(MODULES),$(eval obj-m += $(m:.ko=.o)))
>  
>  CFLAGS_bpf_testmod.o = -I$(src)
>  
> +# When BPF_STRICT_BUILD != 0, a missing KDIR is fatal (the default).
> +# When permissive, skip silently.
> +PERMISSIVE := $(filter 0,$(BPF_STRICT_BUILD))
> +
>  all:
> -	$(Q)$(MAKE) -C $(KDIR) M=$(TEST_KMOD_DIR) modules
> +ifeq ($(PERMISSIVE),)
> +	$(Q)$(MAKE) -C $(KDIR) $(if $(KMOD_O_VALID),O=$(KMOD_O_VALID),KBUILD_OUTPUT=) \

Will this cause an issue with Kbuild variable precedence if both O= and
KBUILD_OUTPUT= are set to different directories?

When a user provides both O= on the command line and KBUILD_OUTPUT= in the
environment, standard Kbuild prioritizes O=.

However, the parent bpf/Makefile passes KBUILD_OUTPUT explicitly on the
command line to this sub-make, upgrading it to a command-line variable
stored in MAKEFLAGS. This sub-make correctly evaluates
$(or $(O),$(KBUILD_OUTPUT)) to validate the O= directory, and calls the
kernel Makefile passing O=$(KMOD_O_VALID).

But because KBUILD_OUTPUT is also a command-line variable in MAKEFLAGS,
the kernel's top-level Makefile assignment "KBUILD_OUTPUT := $(O)" is
ignored by GNU Make. Consequently, the kernel build uses the directory
specified by KBUILD_OUTPUT rather than the validated O= directory. This
leads to a build failure if KBUILD_OUTPUT lacks the prepared build
infrastructure.

Could we explicitly override KBUILD_OUTPUT when KMOD_O_VALID is true?
Something like:

    $(if $(KMOD_O_VALID),O=$(KMOD_O_VALID) \
        KBUILD_OUTPUT=$(KMOD_O_VALID),KBUILD_OUTPUT=)

> +		M=$(TEST_KMOD_DIR) modules
> +else ifneq ("$(wildcard $(KDIR))", "")
> +	$(Q)$(MAKE) -C $(KDIR) $(if $(KMOD_O_VALID),O=$(KMOD_O_VALID),KBUILD_OUTPUT=) \

Would the same explicit override for KBUILD_OUTPUT be needed here as well?

> +		M=$(TEST_KMOD_DIR) modules
> +endif
>  
>  clean:
> -	$(Q)$(MAKE) -C $(KDIR) M=$(TEST_KMOD_DIR) clean
> +ifneq ("$(wildcard $(KDIR))", "")
> +	$(Q)$(MAKE) -C $(KDIR) $(if $(KMOD_O_VALID),O=$(KMOD_O_VALID),KBUILD_OUTPUT=) \

And also for the clean target?

> +		M=$(TEST_KMOD_DIR) clean
> +endif

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260428-selftests-bpf_misconfig-v8-0-bf02cf97dbcb@suse.com?part=2