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