From: sashiko-bot@kernel.org
To: "Eduard Zingerman" <eddyz87@gmail.com>
Cc: bpf@vger.kernel.org
Subject: Re: [PATCH bpf-next v3 2/2] selftests/bpf: verify kallsyms entries for token-loaded subprograms
Date: Wed, 15 Apr 2026 19:17:37 +0000 [thread overview]
Message-ID: <20260415191738.226BFC19424@smtp.kernel.org> (raw)
In-Reply-To: <20260415-subprog-token-fix-v3-2-6fefe1d51646@gmail.com>
Sashiko AI review found 1 potential issue(s):
- [Medium] Unchecked return values of `sysctl_set()` in `serial_test_token()` can lead to confusing test failures.
--
commit e7dc629b9f37dc08742ff9bbddeaaf65c1e34956
Author: Eduard Zingerman <eddyz87@gmail.com>
selftests/bpf: verify kallsyms entries for token-loaded subprograms
This commit adds a test to verify that both the main XDP program and its
global subprogram appear in /proc/kallsyms when loaded using a user
namespace BPF token. It also introduces a sysctl_set utility to configure
system parameters needed to read kallsyms.
> diff --git a/tools/testing/selftests/bpf/prog_tests/token.c b/tools/testing/selftests/bpf/prog_tests/token.c
> index b81dde283052e..cf5b94512cff4 100644
> --- a/tools/testing/selftests/bpf/prog_tests/token.c
> +++ b/tools/testing/selftests/bpf/prog_tests/token.c
[ ... ]
> @@ -1194,4 +1252,23 @@ void test_token(void)
>
> subtest_userns(&opts, userns_bpf_token_info);
> }
> + if (test__start_subtest("obj_priv_prog_kallsyms")) {
> + char perf_paranoid_orig[32] = {};
> + char kptr_restrict_orig[32] = {};
> + struct bpffs_opts opts = {
> + .cmds = bit(BPF_BTF_LOAD) | bit(BPF_PROG_LOAD),
> + .progs = bit(BPF_PROG_TYPE_XDP),
> + .attachs = ~0ULL,
> + };
> +
> + sysctl_set("/proc/sys/kernel/perf_event_paranoid", perf_paranoid_orig, "0");
> + sysctl_set("/proc/sys/kernel/kptr_restrict", kptr_restrict_orig, "0");
Should the return values of these sysctl_set calls be checked?
If the test is executed in an environment where these sysctls are read-only
or inaccessible (such as inside a container), sysctl_set will silently fail.
This would cause load_kallsyms_local to read zeroed addresses because
kptr_restrict wasn't successfully disabled, resulting in confusing test
failures later at ASSERT_TRUE instead of catching the setup failure early.
Would it be better to wrap these with ASSERT_OK?
> +
> + subtest_userns(&opts, userns_obj_priv_prog_kallsyms);
> +
> + if (perf_paranoid_orig[0])
> + sysctl_set("/proc/sys/kernel/perf_event_paranoid", NULL, perf_paranoid_orig);
> + if (kptr_restrict_orig[0])
> + sysctl_set("/proc/sys/kernel/kptr_restrict", NULL, kptr_restrict_orig);
> + }
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260415-subprog-token-fix-v3-0-6fefe1d51646@gmail.com?part=2
next prev parent reply other threads:[~2026-04-15 19:17 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-04-15 18:35 [PATCH bpf-next v3 0/2] bpf: copy BPF token from main program to subprograms Eduard Zingerman
2026-04-15 18:35 ` [PATCH bpf-next v3 1/2] " Eduard Zingerman
2026-04-15 18:35 ` [PATCH bpf-next v3 2/2] selftests/bpf: verify kallsyms entries for token-loaded subprograms Eduard Zingerman
2026-04-15 19:17 ` sashiko-bot [this message]
2026-04-15 19:32 ` Eduard Zingerman
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=20260415191738.226BFC19424@smtp.kernel.org \
--to=sashiko-bot@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=eddyz87@gmail.com \
--cc=sashiko@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox