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 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.