All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Ricardo B. Marlière (SUSE)" <ricardo@marliere.net>
Cc: bpf@vger.kernel.org
Subject: Re: [PATCH 1/3] selftests/bpf: Route generated test headers to OUTPUT
Date: Wed, 29 Jul 2026 00:16:20 +0000	[thread overview]
Message-ID: <20260729001621.34FA31F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260728-selftests-bpf_oot-v1-1-05feb15d94db@marliere.net>

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Unescaped path injection in `sed` with the `@` delimiter can cause verifier tests to be silently skipped if the build path contains an `@` character.
- [Low] Generated test headers moved to $(OUTPUT) are not cleaned during out-of-tree builds because EXTRA_CLEAN is not updated.
--

--- Patch [1]: [PATCH 1/3] selftests/bpf: Route generated test headers to OUTPUT ---
commit 63d1e1deab06130ae6d45a6e98fd6e8bf85c23ef
Author: Ricardo B. Marlière (SUSE) <ricardo@marliere.net>

selftests/bpf: Route generated test headers to OUTPUT

This commit updates the Makefile to write generated test-runner enumeration
headers, verifier test lists, and signing certificate headers to $(OUTPUT)
instead of the source tree during out-of-tree builds. It modifies the
target prerequisites to ensure the output subdirectories are created before
generating these headers.

> diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> index b642ee489ea64..b289183475d4c 100644
> --- a/tools/testing/selftests/bpf/Makefile
> +++ b/tools/testing/selftests/bpf/Makefile

[ ... ]

> @@ -923,14 +923,14 @@ $(eval $(call DEFINE_TEST_RUNNER,test_maps))
>  # It is much simpler than test_maps/test_progs and sufficiently different from
>  # them (e.g., test.h is using completely pattern), that it's worth just
>  # explicitly defining all the rules explicitly.
> -verifier/tests.h: verifier/*.c
> -	$(shell ( cd verifier/; \
> +$(OUTPUT)/verifier/tests.h: verifier/*.c | $(OUTPUT)
> +	$(shell mkdir -p $(OUTPUT)/verifier && ( \
>  		  echo '/* Generated header, do not edit */'; \
>  		  echo '#ifdef FILL_ARRAY'; \
> -		  ls *.c 2> /dev/null | sed -e 's@\(.*\)@#include \"\1\"@'; \
> +		  ls verifier/*.c 2> /dev/null | sed -e 's@\(.*\)@#include "$(CURDIR)/\1"@'; \

[Severity: Medium]
Does this change introduce a regression if the build path $(CURDIR) contains
an @ character?

If the user's working directory contains an @ symbol (such as in Jenkins
workspaces like @tmp), sed fails with a syntax error because @ is used
as the replacement delimiter.

Since the failure occurs within a subshell (...) > $(OUTPUT)/verifier/tests.h,
Make ignores the failure and writes an empty test array to the header.

Will this cause the verifier test suite to compile successfully but run zero
tests?

>  		  echo '#endif' \
> -		) > verifier/tests.h)
> -$(OUTPUT)/test_verifier: test_verifier.c verifier/tests.h $(BPFOBJ) | $(OUTPUT)
> +		) > $(OUTPUT)/verifier/tests.h)
> +$(OUTPUT)/test_verifier: test_verifier.c $(OUTPUT)/verifier/tests.h $(BPFOBJ) | $(OUTPUT)
>  	$(call msg,BINARY,,$@)
>  	$(Q)$(CC) $(CFLAGS) $(filter %.a %.o %.c,$^) $(LDLIBS) -o $@

[Severity: Low]
With the generated test headers moving to $(OUTPUT), will they be left behind
during a clean build?

Looking at EXTRA_CLEAN further down in tools/testing/selftests/bpf/Makefile:

EXTRA_CLEAN := $(SCRATCH_DIR) $(HOST_SCRATCH_DIR)			\
	prog_tests/tests.h map_tests/tests.h verifier/tests.h		\
	...

Since EXTRA_CLEAN still hardcodes the old relative paths, does this mean
`make clean O=out` will miss the newly relocated headers in the output
directory?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260728-selftests-bpf_oot-v1-0-05feb15d94db@marliere.net?part=1

  reply	other threads:[~2026-07-29  0:16 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-29  0:06 [PATCH 0/3] selftests/bpf: Fixes for out-of-tree builds Ricardo B. Marlière (SUSE)
2026-07-29  0:06 ` [PATCH 1/3] selftests/bpf: Route generated test headers to OUTPUT Ricardo B. Marlière (SUSE)
2026-07-29  0:16   ` sashiko-bot [this message]
2026-07-29  0:06 ` [PATCH 2/3] selftests/bpf: Route libarena build artifacts " Ricardo B. Marlière (SUSE)
2026-07-29  0:18   ` sashiko-bot
2026-07-29  0:06 ` [PATCH 3/3] selftests/bpf: Route test_kmods " Ricardo B. Marlière (SUSE)

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260729001621.34FA31F000E9@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=ricardo@marliere.net \
    --cc=sashiko-reviews@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.