From: "Emil Tsalapatis" <emil@etsalapatis.com>
To: "Kumar Kartikeya Dwivedi" <memxor@gmail.com>, <bpf@vger.kernel.org>
Cc: "Tejun Heo" <tj@kernel.org>,
"Dan Schatzberg" <dschatzberg@meta.com>,
"Alexei Starovoitov" <ast@kernel.org>,
"Andrii Nakryiko" <andrii@kernel.org>,
"Daniel Borkmann" <daniel@iogearbox.net>,
"Martin KaFai Lau" <martin.lau@kernel.org>,
"Eduard Zingerman" <eddyz87@gmail.com>, <kkd@meta.com>,
<kernel-team@meta.com>
Subject: Re: [PATCH bpf-next v1 1/4] bpf: Support variable offsets for syscall PTR_TO_CTX
Date: Tue, 17 Mar 2026 12:45:32 -0400 [thread overview]
Message-ID: <DH57JO6V5L54.I3CL1UF0FKAE@etsalapatis.com> (raw)
In-Reply-To: <20260317111850.2107846-2-memxor@gmail.com>
On Tue Mar 17, 2026 at 7:18 AM EDT, Kumar Kartikeya Dwivedi wrote:
> Allow accessing PTR_TO_CTX with variable offsets in syscall programs.
> Fixed offsets are already enabled for all program types that do not
> convert their ctx accesses, since the changes we made in the commit
> de6c7d99f898 ("bpf: Relax fixed offset check for PTR_TO_CTX"). Note
> that we also lift the restriction on passing syscall context into
> helpers, which was not permitted before, and passing modified syscall
> context into kfuncs.
>
> The structure of check_mem_access can be mostly shared and preserved,
> but we must use check_mem_region_access to correctly verify access with
> variable offsets.
>
> The check made in check_helper_mem_access is hardened to only allow
> PTR_TO_CTX for syscall programs to be passed in as helper memory. This
> was the original intention of the existing code anyway, and it makes
> little sense for other program types' context to be utilized as a memory
> buffer. In case a convincing example presents itself in the future, this
> check can be relaxed further.
>
> We also no longer use the last-byte access to simulate helper memory
> access, but instead go through check_mem_region_access. Since this no
> longer updates our max_ctx_offset, we must do so manually, to keep track
> of the maximum offset at which the program ctx may be accessed.
>
> Cc: Tejun Heo <tj@kernel.org>
> Cc: Dan Schatzberg <dschatzberg@meta.com>
> Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
> ---
One nit (along with the bot's point which seems correct, patch 3 can be rolled
into this one). Given those:
Reviewed-by: Emil Tsalapatis <emil@etsalapatis.com>
> kernel/bpf/verifier.c | 51 +++++++++++--------
> .../bpf/progs/verifier_global_subprogs.c | 1 -
> 2 files changed, 31 insertions(+), 21 deletions(-)
>
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 01c18f4268de..50639bb69d91 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -7843,6 +7843,7 @@ static int check_mem_access(struct bpf_verifier_env *env, int insn_idx, u32 regn
> * Program types that don't rewrite ctx accesses can safely
> * dereference ctx pointers with fixed offsets.
> */
> + bool var_off_ok = resolve_prog_type(env->prog) == BPF_PROG_TYPE_SYSCALL;
> bool fixed_off_ok = !env->ops->convert_ctx_access;
> struct bpf_retval_range range;
> struct bpf_insn_access_aux info = {
> @@ -7857,15 +7858,25 @@ static int check_mem_access(struct bpf_verifier_env *env, int insn_idx, u32 regn
> return -EACCES;
> }
>
> - err = __check_ptr_off_reg(env, reg, regno, fixed_off_ok);
> - if (err < 0)
> - return err;
> + if (var_off_ok) {
> + err = check_mem_region_access(env, regno, off, size, U16_MAX, false);
> + if (err)
> + return err;
> + } else {
> + err = __check_ptr_off_reg(env, reg, regno, fixed_off_ok);
> + if (err < 0)
> + return err;
> + }
>
> /*
> * Fold the register's constant offset into the insn offset so
> - * that is_valid_access() sees the true effective offset.
> + * that is_valid_access() sees the true effective offset. If the
> + * register's offset is not constant, then the maximum possible
> + * offset is simulated.
> */
> - if (fixed_off_ok)
> + if (var_off_ok)
> + off += reg->umax_value;
> + else if (fixed_off_ok)
Nit: Can we move the offset adjustment into the if-else above? We're
essentially repeating the if (var_off_ok) check twice.
> off += reg->var_off.value;
> err = check_ctx_access(env, insn_idx, off, size, t, &info);
> if (err)
> @@ -8442,22 +8453,16 @@ static int check_helper_mem_access(struct bpf_verifier_env *env, int regno,
> return check_ptr_to_btf_access(env, regs, regno, 0,
> access_size, BPF_READ, -1);
> case PTR_TO_CTX:
> - /* in case the function doesn't know how to access the context,
> - * (because we are in a program of type SYSCALL for example), we
> - * can not statically check its size.
> - * Dynamically check it now.
> - */
> - if (!env->ops->convert_ctx_access) {
> - int offset = access_size - 1;
> -
> - /* Allow zero-byte read from PTR_TO_CTX */
> - if (access_size == 0)
> - return zero_size_allowed ? 0 : -EACCES;
> -
> - return check_mem_access(env, env->insn_idx, regno, offset, BPF_B,
> - access_type, -1, false, false);
> + /* Only permit reading or writing syscall context using helper calls. */
> + if (resolve_prog_type(env->prog) == BPF_PROG_TYPE_SYSCALL) {
> + int err = check_mem_region_access(env, regno, 0, access_size, U16_MAX,
> + zero_size_allowed);
> + if (err)
> + return err;
> + if (env->prog->aux->max_ctx_offset < reg->umax_value + access_size)
> + env->prog->aux->max_ctx_offset = reg->umax_value + access_size;
> + return 0;
> }
> -
> fallthrough;
> default: /* scalar_value or invalid ptr */
> /* Allow zero-byte read from NULL, regardless of pointer type */
> @@ -9401,6 +9406,7 @@ static const struct bpf_reg_types mem_types = {
> PTR_TO_MEM | MEM_RINGBUF,
> PTR_TO_BUF,
> PTR_TO_BTF_ID | PTR_TRUSTED,
> + PTR_TO_CTX,
> },
> };
>
> @@ -9710,6 +9716,11 @@ static int check_func_arg_reg_off(struct bpf_verifier_env *env,
> * still need to do checks instead of returning.
> */
> return __check_ptr_off_reg(env, reg, regno, true);
> + case PTR_TO_CTX:
> + /* Allow fixed and variable offsets for syscall context. */
> + if (resolve_prog_type(env->prog) == BPF_PROG_TYPE_SYSCALL)
> + return 0;
> + fallthrough;
> default:
> return __check_ptr_off_reg(env, reg, regno, false);
> }
> diff --git a/tools/testing/selftests/bpf/progs/verifier_global_subprogs.c b/tools/testing/selftests/bpf/progs/verifier_global_subprogs.c
> index f02012a2fbaa..2250fc31574d 100644
> --- a/tools/testing/selftests/bpf/progs/verifier_global_subprogs.c
> +++ b/tools/testing/selftests/bpf/progs/verifier_global_subprogs.c
> @@ -134,7 +134,6 @@ __noinline __weak int subprog_user_anon_mem(user_struct_t *t)
>
> SEC("?tracepoint")
> __failure __log_level(2)
> -__msg("invalid bpf_context access")
> __msg("Caller passes invalid args into func#1 ('subprog_user_anon_mem')")
> int anon_user_mem_invalid(void *ctx)
> {
next prev parent reply other threads:[~2026-03-17 16:45 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-03-17 11:18 [PATCH bpf-next v1 0/4] Allow variable offsets for syscall PTR_TO_CTX Kumar Kartikeya Dwivedi
2026-03-17 11:18 ` [PATCH bpf-next v1 1/4] bpf: Support " Kumar Kartikeya Dwivedi
2026-03-17 12:09 ` bot+bpf-ci
2026-03-17 16:45 ` Emil Tsalapatis [this message]
2026-03-17 11:18 ` [PATCH bpf-next v1 2/4] selftests/bpf: Adjust syscall ctx variable offset tests Kumar Kartikeya Dwivedi
2026-03-17 18:24 ` Emil Tsalapatis
2026-03-17 11:18 ` [PATCH bpf-next v1 3/4] bpf: Reject modified syscall PTR_TO_CTX for global subprogs Kumar Kartikeya Dwivedi
2026-03-17 16:50 ` Emil Tsalapatis
2026-03-17 18:46 ` Kumar Kartikeya Dwivedi
2026-03-17 11:18 ` [PATCH bpf-next v1 4/4] selftests/bpf: Test modified syscall ctx for global subprog Kumar Kartikeya Dwivedi
2026-03-17 16:13 ` Emil Tsalapatis
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=DH57JO6V5L54.I3CL1UF0FKAE@etsalapatis.com \
--to=emil@etsalapatis.com \
--cc=andrii@kernel.org \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=dschatzberg@meta.com \
--cc=eddyz87@gmail.com \
--cc=kernel-team@meta.com \
--cc=kkd@meta.com \
--cc=martin.lau@kernel.org \
--cc=memxor@gmail.com \
--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.