From: sashiko-bot@kernel.org
To: "Tejun Heo" <tj@kernel.org>
Cc: sched-ext@lists.linux.dev, bpf@vger.kernel.org
Subject: Re: [PATCHSET SLOP RFC 2/6] selftests/bpf: Add kfunc __arena argument tests
Date: Mon, 13 Jul 2026 03:16:57 +0000 [thread overview]
Message-ID: <20260713031657.7ECAB1F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260713024414.3759854-3-tj@kernel.org>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The kfunc arena argument fixup logic incorrectly translates a valid pointer to the first page of a 4GB-aligned arena as NULL, causing kfuncs to receive a NULL pointer instead of a valid kernel address.
- [Low] Multi-line comment does not follow the mandated BPF subsystem style.
--
--- Patch [2]: [PATCHSET SLOP RFC 2/6] selftests/bpf: Add kfunc __arena argument tests ---
commit c3a66495e39c4d671a1065fb44bc6cef2f26d120
Author: Tejun Heo <tj@kernel.org>
selftests/bpf: Add kfunc __arena argument tests
Add arena-argument kfuncs to bpf_testmod, which also exercises the
argument rebasing on module kfuncs, and tests covering the accepted
argument forms (arena pointer, low 32 bits as a scalar, full user address
as a scalar, NULL), five arena arguments in one call with NULLs mixed in,
a kernel-side dereference of an unpopulated page recovering through the
scratch page, and the rejections (no arena in the program, incompatible
register type).
> diff --git a/tools/testing/selftests/bpf/prog_tests/arena_kfunc.c b/tools/testing/selftests/bpf/prog_tests/arena_kfunc.c
> new file mode 100644
> index 0000000000000..8946184553ea8
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/arena_kfunc.c
> @@ -0,0 +1,14 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (c) 2026 Tejun Heo <tj@kernel.org> */
> +#include <test_progs.h>
> +
> +#include "arena_kfunc.skel.h"
> +
> +/* The test kfuncs live in bpf_testmod. Resolving kfuncs against module
> + * BTFs needs CAP_SYS_ADMIN, so run with full capabilities instead of
> + * through the verifier tests' capability-restricted runner.
> + */
[Severity: Low]
This isn't a bug, but does this multi-line comment follow the BPF
subsystem style?
The opening /* should be on its own line:
/*
* The test kfuncs live in bpf_testmod...
*/
> +void test_arena_kfunc(void)
> +{
> + RUN_TESTS(arena_kfunc);
> +}
[ ... ]
> diff --git a/tools/testing/selftests/bpf/progs/arena_kfunc.c b/tools/testing/selftests/bpf/progs/arena_kfunc.c
> new file mode 100644
> index 0000000000000..b5996944ed73b
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/arena_kfunc.c
[ ... ]
> + /* NULL is preserved */
> + ret = bpf_kfunc_arena_arg_test(NULL);
> + if (ret != 0xdeadbeef)
> + return 5;
[Severity: High]
Does the kfunc arena argument fixup logic tested here incorrectly translate
a valid pointer to the first page of a 4GB-aligned arena as NULL?
In kernel/bpf/verifier.c:bpf_fixup_kfunc_call(), if an arena is created with
a 4GB-aligned user_vm_start, and a BPF program passes a pointer to offset 0
within this arena to a kfunc, the fixup logic uses BPF_ZEXT_REG(regno) to
clear the upper 32 bits, making the register exactly 0.
This is then explicitly checked by BPF_JMP_IMM(BPF_JEQ, regno, 0, 1), which
skips the kern_vm_start base addition.
Would this cause kfuncs to receive a NULL pointer instead of a valid kernel
address when passed a pointer to offset 0?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260713024414.3759854-1-tj@kernel.org?part=2
next prev parent reply other threads:[~2026-07-13 3:16 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-07-13 2:44 [PATCHSET SLOP RFC 0/6] bpf: make arena pointers first-class kfunc and struct_ops arguments Tejun Heo
2026-07-13 2:44 ` [PATCHSET SLOP RFC 1/6] bpf: Support __arena suffix for kfunc arguments Tejun Heo
2026-07-13 2:58 ` sashiko-bot
2026-07-13 19:38 ` Tejun Heo
2026-07-13 21:45 ` Kumar Kartikeya Dwivedi
2026-07-13 2:44 ` [PATCHSET SLOP RFC 2/6] selftests/bpf: Add kfunc __arena argument tests Tejun Heo
2026-07-13 3:16 ` sashiko-bot [this message]
2026-07-13 19:38 ` Tejun Heo
2026-07-13 2:44 ` [PATCHSET SLOP RFC 3/6] bpf: Support __arena suffix on struct_ops stub arguments Tejun Heo
2026-07-13 2:59 ` sashiko-bot
2026-07-13 19:37 ` [PATCH v2 " Tejun Heo
2026-07-13 2:44 ` [PATCHSET SLOP RFC 4/6] selftests/bpf: Add struct_ops __arena argument tests Tejun Heo
2026-07-13 2:55 ` sashiko-bot
2026-07-13 19:38 ` Tejun Heo
2026-07-13 2:44 ` [PATCHSET SLOP RFC 5/6] sched_ext: Pass a kernel arena pointer to ops_cid.set_cmask() Tejun Heo
2026-07-13 2:44 ` [PATCHSET SLOP RFC 6/6] sched_ext: Convert scx_bpf_cid_override() to take an arena pointer Tejun Heo
2026-07-13 2:59 ` sashiko-bot
2026-07-13 19:38 ` Tejun Heo
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=20260713031657.7ECAB1F000E9@smtp.kernel.org \
--to=sashiko-bot@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=sashiko-reviews@lists.linux.dev \
--cc=sched-ext@lists.linux.dev \
--cc=tj@kernel.org \
/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