[PATCH bpf-next v3 0/3] Allow variable offsets for syscall PTR_TO_CTX

[PATCH bpf-next v3 0/3] Allow variable offsets for syscall PTR_TO_CTX

[Kumar Kartikeya Dwivedi]

Enable pointer modification with variable offsets accumulated in the
register for PTR_TO_CTX for syscall programs where it won't be
rewritten, and the context is user-supplied and checked against the max
offset. See patches for details. Fixed offset support landed in [0].

By combining this set with [0], examples like the one below should
succeed verification now.

  SEC("syscall")
  int prog(void *ctx) {
	int *arr = ctx;
	int i;

	bpf_for(i, 0, 100)
		arr[i] *= i;

	return 0;
  }

  [0]: https://lore.kernel.org/bpf/20260227005725.1247305-1-memxor@gmail.com

Changelog:
----------
v2 -> v3
v2: https://lore.kernel.org/bpf/20260318075133.1031781-1-memxor@gmail.com

 * Prevent arg_type for KF_ARG_PTR_TO_CTX from applying to other cases
   due to preceding fallthrough. (Gemini/Sashiko)

v1 -> v2
v1: https://lore.kernel.org/bpf/20260317111850.2107846-2-memxor@gmail.com

 * Harden check_func_arg_reg_off check with ARG_PTR_TO_CTX.
 * Add tests for unmodified ctx into tail calls.
 * Squash unmodified ctx change into base commit.
 * Add Reviewed-by's from Emil.

Kumar Kartikeya Dwivedi (3):
  bpf: Support variable offsets for syscall PTR_TO_CTX
  selftests/bpf: Add syscall ctx variable offset tests
  selftests/bpf: Test modified syscall ctx for ARG_PTR_TO_CTX

 kernel/bpf/verifier.c                         |  67 ++--
 .../selftests/bpf/prog_tests/verifier.c       |  34 +-
 .../selftests/bpf/progs/verifier_ctx.c        | 356 +++++++++++++++---
 .../bpf/progs/verifier_global_subprogs.c      |  95 ++++-
 .../selftests/bpf/test_kmods/bpf_testmod.c    |   2 +-
 5 files changed, 462 insertions(+), 92 deletions(-)


base-commit: 77378dabb50f593c756d393d8eacb0b91b758863
-- 
2.52.0

[PATCH bpf-next v3 1/3] bpf: Support variable offsets for syscall PTR_TO_CTX

[Kumar Kartikeya Dwivedi]

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.

Take care to ensure that when arg_type is ARG_PTR_TO_CTX, we do not
relax any fixed or variable offset constraints around PTR_TO_CTX even in
syscall programs, and require them to be passed unmodified. There are
several reasons why this is necessary. First, if we pass a modified ctx,
then the global subprog's accesses will not update the max_ctx_offset to
its true maximum offset, and can lead to out of bounds accesses. Second,
tail called program (or extension program replacing global subprog) where
their max_ctx_offset exceeds the program they are being called from can
also cause issues. For the latter, unmodified PTR_TO_CTX is the first
requirement for the fix, the second is ensuring max_ctx_offset >= the
program they are being called from, which has to be a separate change
not made in this commit.

All in all, we can hint using arg_type when we expect ARG_PTR_TO_CTX and
make our relaxation around offsets conditional on it.

Cc: Tejun Heo <tj@kernel.org>
Cc: Dan Schatzberg <dschatzberg@meta.com>
Reviewed-by: Emil Tsalapatis <emil@etsalapatis.com>
Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
---
 kernel/bpf/verifier.c                         | 67 ++++++++++++-------
 .../bpf/progs/verifier_global_subprogs.c      |  1 -
 2 files changed, 43 insertions(+), 25 deletions(-)

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 01c18f4268de..14bf64e0c600 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,16 +7858,26 @@ 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;
-
 		/*
 		 * 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 allowed to be variable, then the maximum
+		 * possible offset is simulated (which is equal to var_off.value
+		 * when var_off is constant).
 		 */
-		if (fixed_off_ok)
-			off += reg->var_off.value;
+		if (var_off_ok) {
+			err = check_mem_region_access(env, regno, off, size, U16_MAX, false);
+			if (err)
+				return err;
+			off += reg->umax_value;
+		} else {
+			err = __check_ptr_off_reg(env, reg, regno, fixed_off_ok);
+			if (err < 0)
+				return err;
+			if (fixed_off_ok)
+				off += reg->var_off.value;
+		}
+
 		err = check_ctx_access(env, insn_idx, off, size, t, &info);
 		if (err)
 			verbose_linfo(env, insn_idx, "; ");
@@ -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,17 @@ 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, but
+		 * only when the argument is passed as memory, not ctx,
+		 * otherwise we may get modified ctx in tail called programs and
+		 * global subprogs (that may act as extension prog hooks).
+		 */
+		if (arg_type != ARG_PTR_TO_CTX &&
+		    resolve_prog_type(env->prog) == BPF_PROG_TYPE_SYSCALL)
+			return 0;
+		fallthrough;
 	default:
 		return __check_ptr_off_reg(env, reg, regno, false);
 	}
@@ -10757,7 +10774,7 @@ static int btf_check_func_arg_match(struct bpf_verifier_env *env, int subprog,
 			 * invalid memory access.
 			 */
 		} else if (arg->arg_type == ARG_PTR_TO_CTX) {
-			ret = check_func_arg_reg_off(env, reg, regno, ARG_DONTCARE);
+			ret = check_func_arg_reg_off(env, reg, regno, ARG_PTR_TO_CTX);
 			if (ret < 0)
 				return ret;
 			/* If function expects ctx type in BTF check that caller
@@ -13565,7 +13582,6 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_kfunc_call_
 				}
 			}
 			fallthrough;
-		case KF_ARG_PTR_TO_CTX:
 		case KF_ARG_PTR_TO_DYNPTR:
 		case KF_ARG_PTR_TO_ITER:
 		case KF_ARG_PTR_TO_LIST_HEAD:
@@ -13583,6 +13599,9 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_kfunc_call_
 		case KF_ARG_PTR_TO_IRQ_FLAG:
 		case KF_ARG_PTR_TO_RES_SPIN_LOCK:
 			break;
+		case KF_ARG_PTR_TO_CTX:
+			arg_type = ARG_PTR_TO_CTX;
+			break;
 		default:
 			verifier_bug(env, "unknown kfunc arg type %d", kf_arg_type);
 			return -EFAULT;
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)
 {
-- 
2.52.0

[PATCH bpf-next v3 2/3] selftests/bpf: Add syscall ctx variable offset tests

[Kumar Kartikeya Dwivedi]

Add various tests to exercise fixed and variable offsets on PTR_TO_CTX
for syscall programs, and cover disallowed cases for other program types
lacking convert_ctx_access callback. Load verifier_ctx with CAP_SYS_ADMIN
so that kfunc related logic can be tested. While at it, convert assembly
tests to C.

Reviewed-by: Emil Tsalapatis <emil@etsalapatis.com>
Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
---
 .../selftests/bpf/prog_tests/verifier.c       |  34 +-
 .../selftests/bpf/progs/verifier_ctx.c        | 356 +++++++++++++++---
 .../selftests/bpf/test_kmods/bpf_testmod.c    |   2 +-
 3 files changed, 325 insertions(+), 67 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/verifier.c b/tools/testing/selftests/bpf/prog_tests/verifier.c
index 8cdfd74c95d7..04d5f46264a3 100644
--- a/tools/testing/selftests/bpf/prog_tests/verifier.c
+++ b/tools/testing/selftests/bpf/prog_tests/verifier.c
@@ -126,29 +126,35 @@ struct test_val {
 __maybe_unused
 static void run_tests_aux(const char *skel_name,
 			  skel_elf_bytes_fn elf_bytes_factory,
-			  pre_execution_cb pre_execution_cb)
+			  pre_execution_cb pre_execution_cb,
+			  bool drop_sys_admin)
 {
 	struct test_loader tester = {};
 	__u64 old_caps;
 	int err;
 
-	/* test_verifier tests are executed w/o CAP_SYS_ADMIN, do the same here */
-	err = cap_disable_effective(1ULL << CAP_SYS_ADMIN, &old_caps);
-	if (err) {
-		PRINT_FAIL("failed to drop CAP_SYS_ADMIN: %i, %s\n", err, strerror(-err));
-		return;
+	if (drop_sys_admin) {
+		/* test_verifier tests are executed w/o CAP_SYS_ADMIN, do the same here */
+		err = cap_disable_effective(1ULL << CAP_SYS_ADMIN, &old_caps);
+		if (err) {
+			PRINT_FAIL("failed to drop CAP_SYS_ADMIN: %i, %s\n", err, strerror(-err));
+			return;
+		}
 	}
 
 	test_loader__set_pre_execution_cb(&tester, pre_execution_cb);
 	test_loader__run_subtests(&tester, skel_name, elf_bytes_factory);
 	test_loader_fini(&tester);
 
-	err = cap_enable_effective(old_caps, NULL);
-	if (err)
-		PRINT_FAIL("failed to restore CAP_SYS_ADMIN: %i, %s\n", err, strerror(-err));
+	if (drop_sys_admin) {
+		err = cap_enable_effective(old_caps, NULL);
+		if (err)
+			PRINT_FAIL("failed to restore CAP_SYS_ADMIN: %i, %s\n", err, strerror(-err));
+	}
 }
 
-#define RUN(skel) run_tests_aux(#skel, skel##__elf_bytes, NULL)
+#define RUN(skel) run_tests_aux(#skel, skel##__elf_bytes, NULL, true)
+#define RUN_WITH_CAP_SYS_ADMIN(skel) run_tests_aux(#skel, skel##__elf_bytes, NULL, false)
 
 void test_verifier_align(void)                { RUN(verifier_align); }
 void test_verifier_and(void)                  { RUN(verifier_and); }
@@ -173,7 +179,7 @@ void test_verifier_cgroup_skb(void)           { RUN(verifier_cgroup_skb); }
 void test_verifier_cgroup_storage(void)       { RUN(verifier_cgroup_storage); }
 void test_verifier_const(void)                { RUN(verifier_const); }
 void test_verifier_const_or(void)             { RUN(verifier_const_or); }
-void test_verifier_ctx(void)                  { RUN(verifier_ctx); }
+void test_verifier_ctx(void)                  { RUN_WITH_CAP_SYS_ADMIN(verifier_ctx); }
 void test_verifier_ctx_sk_msg(void)           { RUN(verifier_ctx_sk_msg); }
 void test_verifier_d_path(void)               { RUN(verifier_d_path); }
 void test_verifier_default_trusted_ptr(void)  { RUN_TESTS(verifier_default_trusted_ptr); }
@@ -293,7 +299,8 @@ void test_verifier_array_access(void)
 {
 	run_tests_aux("verifier_array_access",
 		      verifier_array_access__elf_bytes,
-		      init_array_access_maps);
+		      init_array_access_maps,
+		      true);
 }
 void test_verifier_async_cb_context(void)    { RUN(verifier_async_cb_context); }
 
@@ -306,5 +313,6 @@ void test_verifier_value_ptr_arith(void)
 {
 	run_tests_aux("verifier_value_ptr_arith",
 		      verifier_value_ptr_arith__elf_bytes,
-		      init_value_ptr_arith_maps);
+		      init_value_ptr_arith_maps,
+		      true);
 }
diff --git a/tools/testing/selftests/bpf/progs/verifier_ctx.c b/tools/testing/selftests/bpf/progs/verifier_ctx.c
index 371780290c0d..c054c8cb7242 100644
--- a/tools/testing/selftests/bpf/progs/verifier_ctx.c
+++ b/tools/testing/selftests/bpf/progs/verifier_ctx.c
@@ -4,6 +4,10 @@
 #include "vmlinux.h"
 #include <bpf/bpf_helpers.h>
 #include "bpf_misc.h"
+#include "../test_kmods/bpf_testmod_kfunc.h"
+
+const char ctx_strncmp_target[] = "ctx";
+const char ctx_snprintf_fmt[] = "";
 
 SEC("tc")
 __description("context stores via BPF_ATOMIC")
@@ -69,7 +73,6 @@ __naked void ctx_pointer_to_helper_1(void)
 SEC("socket")
 __description("pass modified ctx pointer to helper, 2")
 __failure __msg("negative offset ctx ptr R1 off=-612 disallowed")
-__failure_unpriv __msg_unpriv("negative offset ctx ptr R1 off=-612 disallowed")
 __naked void ctx_pointer_to_helper_2(void)
 {
 	asm volatile ("					\
@@ -295,77 +298,324 @@ padding_access("sk_reuseport", sk_reuseport_md, hash, 4);
 SEC("syscall")
 __description("syscall: write to ctx with fixed offset")
 __success
-__naked void syscall_ctx_fixed_off_write(void)
+int syscall_ctx_fixed_off_write(void *ctx)
 {
-	asm volatile ("					\
-	r0 = 0;						\
-	*(u32*)(r1 + 0) = r0;				\
-	r1 += 4;					\
-	*(u32*)(r1 + 0) = r0;				\
-	exit;						\
-"	::: __clobber_all);
+	char *p = ctx;
+
+	*(__u32 *)p = 0;
+	*(__u32 *)(p + 4) = 0;
+	return 0;
+}
+
+SEC("syscall")
+__description("syscall: read ctx with fixed offset")
+__success
+int syscall_ctx_fixed_off_read(void *ctx)
+{
+	char *p = ctx;
+	volatile __u32 val;
+
+	val = *(__u32 *)(p + 4);
+	(void)val;
+	return 0;
+}
+
+SEC("syscall")
+__description("syscall: read ctx with variable offset")
+__success
+int syscall_ctx_var_off_read(void *ctx)
+{
+	__u64 off = bpf_get_prandom_u32();
+	char *p = ctx;
+	volatile __u32 val;
+
+	off &= 0xfc;
+	p += off;
+	val = *(__u32 *)p;
+	(void)val;
+	return 0;
+}
+
+SEC("syscall")
+__description("syscall: write ctx with variable offset")
+__success
+int syscall_ctx_var_off_write(void *ctx)
+{
+	__u64 off = bpf_get_prandom_u32();
+	char *p = ctx;
+
+	off &= 0xfc;
+	p += off;
+	*(__u32 *)p = 0;
+	return 0;
+}
+
+SEC("syscall")
+__description("syscall: reject negative variable offset ctx access")
+__failure __msg("min value is negative")
+int syscall_ctx_neg_var_off(void *ctx)
+{
+	__u64 off = bpf_get_prandom_u32();
+	char *p = ctx;
+
+	off &= 4;
+	p -= off;
+	return *(__u32 *)p;
+}
+
+SEC("syscall")
+__description("syscall: reject unbounded variable offset ctx access")
+__failure __msg("unbounded memory access")
+int syscall_ctx_unbounded_var_off(void *ctx)
+{
+	__u64 off = (__u32)bpf_get_prandom_u32();
+	char *p = ctx;
+
+	off <<= 2;
+	p += off;
+	return *(__u32 *)p;
+}
+
+SEC("syscall")
+__description("syscall: helper read ctx with fixed offset")
+__success
+int syscall_ctx_helper_fixed_off_read(void *ctx)
+{
+	char *p = ctx;
+
+	p += 4;
+	return bpf_strncmp(p, 4, ctx_strncmp_target);
+}
+
+SEC("syscall")
+__description("syscall: helper write ctx with fixed offset")
+__success
+int syscall_ctx_helper_fixed_off_write(void *ctx)
+{
+	char *p = ctx;
+
+	p += 4;
+	return bpf_probe_read_kernel(p, 4, 0);
+}
+
+SEC("syscall")
+__description("syscall: helper read ctx with variable offset")
+__success
+int syscall_ctx_helper_var_off_read(void *ctx)
+{
+	__u64 off = bpf_get_prandom_u32();
+	char *p = ctx;
+
+	off &= 0xfc;
+	p += off;
+	return bpf_strncmp(p, 4, ctx_strncmp_target);
+}
+
+SEC("syscall")
+__description("syscall: helper write ctx with variable offset")
+__success
+int syscall_ctx_helper_var_off_write(void *ctx)
+{
+	__u64 off = bpf_get_prandom_u32();
+	char *p = ctx;
+
+	off &= 0xfc;
+	p += off;
+	return bpf_probe_read_kernel(p, 4, 0);
+}
+
+SEC("syscall")
+__description("syscall: helper read zero-sized ctx access")
+__success
+int syscall_ctx_helper_zero_sized_read(void *ctx)
+{
+	return bpf_snprintf(0, 0, ctx_snprintf_fmt, ctx, 0);
+}
+
+SEC("syscall")
+__description("syscall: helper write zero-sized ctx access")
+__success
+int syscall_ctx_helper_zero_sized_write(void *ctx)
+{
+	return bpf_probe_read_kernel(ctx, 0, 0);
+}
+
+SEC("syscall")
+__description("syscall: kfunc access ctx with fixed offset")
+__success
+int syscall_ctx_kfunc_fixed_off(void *ctx)
+{
+	char *p = ctx;
+
+	p += 4;
+	bpf_kfunc_call_test_mem_len_pass1(p, 4);
+	return 0;
+}
+
+SEC("syscall")
+__description("syscall: kfunc access ctx with variable offset")
+__success
+int syscall_ctx_kfunc_var_off(void *ctx)
+{
+	__u64 off = bpf_get_prandom_u32();
+	char *p = ctx;
+
+	off &= 0xfc;
+	p += off;
+	bpf_kfunc_call_test_mem_len_pass1(p, 4);
+	return 0;
+}
+
+SEC("syscall")
+__description("syscall: kfunc access zero-sized ctx")
+__success
+int syscall_ctx_kfunc_zero_sized(void *ctx)
+{
+	bpf_kfunc_call_test_mem_len_pass1(ctx, 0);
+	return 0;
 }
 
 /*
- * Test that program types without convert_ctx_access can dereference
- * their ctx pointer after adding a fixed offset. Variable and negative
- * offsets should still be rejected.
+ * For non-syscall program types without convert_ctx_access, direct ctx
+ * dereference is still allowed after adding a fixed offset, while variable
+ * and negative direct accesses reject.
+ *
+ * Passing ctx as a helper or kfunc memory argument is only permitted for
+ * syscall programs, so the helper and kfunc cases below validate rejection
+ * for non-syscall ctx pointers at fixed, variable, and zero-sized accesses.
  */
-#define no_rewrite_ctx_access(type, name, off, ld_op)			\
+#define no_rewrite_ctx_access(type, name, off, load_t)			\
 	SEC(type)							\
 	__description(type ": read ctx at fixed offset")		\
 	__success							\
-	__naked void no_rewrite_##name##_fixed(void)			\
+	int no_rewrite_##name##_fixed(void *ctx)			\
 	{								\
-		asm volatile ("						\
-		r1 += %[__off];						\
-		r0 = *(" #ld_op " *)(r1 + 0);				\
-		r0 = 0;							\
-		exit;"							\
-		:							\
-		: __imm_const(__off, off)				\
-		: __clobber_all);					\
+		char *p = ctx;						\
+		volatile load_t val;					\
+									\
+		val = *(load_t *)(p + off);				\
+		(void)val;						\
+		return 0;						\
 	}								\
 	SEC(type)							\
 	__description(type ": reject variable offset ctx access")	\
 	__failure __msg("variable ctx access var_off=")			\
-	__naked void no_rewrite_##name##_var(void)			\
+	int no_rewrite_##name##_var(void *ctx)			\
 	{								\
-		asm volatile ("						\
-		r6 = r1;						\
-		call %[bpf_get_prandom_u32];				\
-		r1 = r6;						\
-		r0 &= 4;						\
-		r1 += r0;						\
-		r0 = *(" #ld_op " *)(r1 + 0);				\
-		r0 = 0;							\
-		exit;"							\
-		:							\
-		: __imm(bpf_get_prandom_u32)				\
-		: __clobber_all);					\
+		__u64 off_var = bpf_get_prandom_u32();			\
+		char *p = ctx;						\
+									\
+		off_var &= 4;						\
+		p += off_var;						\
+		return *(load_t *)p;					\
 	}								\
 	SEC(type)							\
 	__description(type ": reject negative offset ctx access")	\
-	__failure __msg("negative offset ctx ptr")			\
-	__naked void no_rewrite_##name##_neg(void)			\
+	__failure __msg("invalid bpf_context access")			\
+	int no_rewrite_##name##_neg(void *ctx)			\
 	{								\
-		asm volatile ("						\
-		r1 += %[__neg_off];					\
-		r0 = *(" #ld_op " *)(r1 + 0);				\
-		r0 = 0;							\
-		exit;"							\
-		:							\
-		: __imm_const(__neg_off, -(off))			\
-		: __clobber_all);					\
+		char *p = ctx;						\
+									\
+		p -= 612;						\
+		return *(load_t *)p;					\
+	}								\
+	SEC(type)							\
+	__description(type ": reject helper read ctx at fixed offset")	\
+	__failure __msg("dereference of modified ctx ptr")		\
+	int no_rewrite_##name##_helper_read_fixed(void *ctx)		\
+	{								\
+		char *p = ctx;						\
+									\
+		p += off;						\
+		return bpf_strncmp(p, 4, ctx_strncmp_target);		\
+	}								\
+	SEC(type)							\
+	__description(type ": reject helper write ctx at fixed offset")	\
+	__failure __msg("dereference of modified ctx ptr")		\
+	int no_rewrite_##name##_helper_write_fixed(void *ctx)		\
+	{								\
+		char *p = ctx;						\
+									\
+		p += off;						\
+		return bpf_probe_read_kernel(p, 4, 0);			\
+	}								\
+	SEC(type)							\
+	__description(type ": reject helper read ctx with variable offset") \
+	__failure __msg("variable ctx access var_off=")			\
+	int no_rewrite_##name##_helper_read_var(void *ctx)		\
+	{								\
+		__u64 off_var = bpf_get_prandom_u32();			\
+		char *p = ctx;						\
+									\
+		off_var &= 4;						\
+		p += off_var;						\
+		return bpf_strncmp(p, 4, ctx_strncmp_target);		\
+	}								\
+	SEC(type)							\
+	__description(type ": reject helper write ctx with variable offset") \
+	__failure __msg("variable ctx access var_off=")			\
+	int no_rewrite_##name##_helper_write_var(void *ctx)		\
+	{								\
+		__u64 off_var = bpf_get_prandom_u32();			\
+		char *p = ctx;						\
+									\
+		off_var &= 4;						\
+		p += off_var;						\
+		return bpf_probe_read_kernel(p, 4, 0);			\
+	}								\
+	SEC(type)							\
+	__description(type ": reject helper read zero-sized ctx access") \
+	__failure __msg("R4 type=ctx expected=fp")			\
+	int no_rewrite_##name##_helper_read_zero(void *ctx)		\
+	{								\
+		return bpf_snprintf(0, 0, ctx_snprintf_fmt, ctx, 0);	\
+	}								\
+	SEC(type)							\
+	__description(type ": reject helper write zero-sized ctx access") \
+	__failure __msg("R1 type=ctx expected=fp")			\
+	int no_rewrite_##name##_helper_write_zero(void *ctx)		\
+	{								\
+		return bpf_probe_read_kernel(ctx, 0, 0);			\
+	}								\
+	SEC(type)							\
+	__description(type ": reject kfunc ctx at fixed offset")	\
+	__failure __msg("dereference of modified ctx ptr")		\
+	int no_rewrite_##name##_kfunc_fixed(void *ctx)		\
+	{								\
+		char *p = ctx;						\
+									\
+		p += off;						\
+		bpf_kfunc_call_test_mem_len_pass1(p, 4);		\
+		return 0;						\
+	}								\
+	SEC(type)							\
+	__description(type ": reject kfunc ctx with variable offset")	\
+	__failure __msg("variable ctx access var_off=")			\
+	int no_rewrite_##name##_kfunc_var(void *ctx)			\
+	{								\
+		__u64 off_var = bpf_get_prandom_u32();			\
+		char *p = ctx;						\
+									\
+		off_var &= 4;						\
+		p += off_var;						\
+		bpf_kfunc_call_test_mem_len_pass1(p, 4);		\
+		return 0;						\
+	}								\
+	SEC(type)							\
+	__description(type ": reject kfunc zero-sized ctx access")	\
+	__failure __msg("R1 type=ctx expected=fp")			\
+	int no_rewrite_##name##_kfunc_zero(void *ctx)			\
+	{								\
+		bpf_kfunc_call_test_mem_len_pass1(ctx, 0);		\
+		return 0;						\
 	}
 
-no_rewrite_ctx_access("syscall", syscall, 4, u32);
-no_rewrite_ctx_access("kprobe", kprobe, 8, u64);
-no_rewrite_ctx_access("tracepoint", tp, 8, u64);
-no_rewrite_ctx_access("raw_tp", raw_tp, 8, u64);
-no_rewrite_ctx_access("raw_tracepoint.w", raw_tp_w, 8, u64);
-no_rewrite_ctx_access("fentry/bpf_modify_return_test", fentry, 8, u64);
-no_rewrite_ctx_access("cgroup/dev", cgroup_dev, 4, u32);
-no_rewrite_ctx_access("netfilter", netfilter, offsetof(struct bpf_nf_ctx, skb), u64);
+no_rewrite_ctx_access("kprobe", kprobe, 8, __u64);
+no_rewrite_ctx_access("tracepoint", tp, 8, __u64);
+no_rewrite_ctx_access("raw_tp", raw_tp, 8, __u64);
+no_rewrite_ctx_access("raw_tracepoint.w", raw_tp_w, 8, __u64);
+no_rewrite_ctx_access("fentry/bpf_modify_return_test", fentry, 8, __u64);
+no_rewrite_ctx_access("cgroup/dev", cgroup_dev, 4, __u32);
+no_rewrite_ctx_access("netfilter", netfilter, offsetof(struct bpf_nf_ctx, skb), __u64);
 
 char _license[] SEC("license") = "GPL";
diff --git a/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c b/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c
index 94edbd2afa67..f91b484d80f2 100644
--- a/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c
+++ b/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c
@@ -716,6 +716,7 @@ BTF_ID_FLAGS(func, bpf_iter_testmod_seq_next, KF_ITER_NEXT | KF_RET_NULL)
 BTF_ID_FLAGS(func, bpf_iter_testmod_seq_destroy, KF_ITER_DESTROY)
 BTF_ID_FLAGS(func, bpf_iter_testmod_seq_value)
 BTF_ID_FLAGS(func, bpf_kfunc_common_test)
+BTF_ID_FLAGS(func, bpf_kfunc_call_test_mem_len_pass1)
 BTF_ID_FLAGS(func, bpf_kfunc_dynptr_test)
 BTF_ID_FLAGS(func, bpf_kfunc_nested_acquire_nonzero_offset_test, KF_ACQUIRE)
 BTF_ID_FLAGS(func, bpf_kfunc_nested_acquire_zero_offset_test, KF_ACQUIRE)
@@ -1280,7 +1281,6 @@ BTF_ID_FLAGS(func, bpf_kfunc_call_test2)
 BTF_ID_FLAGS(func, bpf_kfunc_call_test3)
 BTF_ID_FLAGS(func, bpf_kfunc_call_test4)
 BTF_ID_FLAGS(func, bpf_kfunc_call_test5)
-BTF_ID_FLAGS(func, bpf_kfunc_call_test_mem_len_pass1)
 BTF_ID_FLAGS(func, bpf_kfunc_call_test_mem_len_fail1)
 BTF_ID_FLAGS(func, bpf_kfunc_call_test_mem_len_fail2)
 BTF_ID_FLAGS(func, bpf_kfunc_call_test_acquire, KF_ACQUIRE | KF_RET_NULL)
-- 
2.52.0

[PATCH bpf-next v3 3/3] selftests/bpf: Test modified syscall ctx for ARG_PTR_TO_CTX

[Kumar Kartikeya Dwivedi]

Ensure that global subprogs and tail calls can only accept an unmodified
PTR_TO_CTX for syscall programs. For all other program types, fixed or
variable offsets on PTR_TO_CTX is rejected when passed into an argument
of any call instruction type, through the unified logic of
check_func_arg_reg_off.

Finally, add a positive example of a case that should succeed with all
our previous changes.

Reviewed-by: Emil Tsalapatis <emil@etsalapatis.com>
Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
---
 .../bpf/progs/verifier_global_subprogs.c      | 94 +++++++++++++++++++
 1 file changed, 94 insertions(+)

diff --git a/tools/testing/selftests/bpf/progs/verifier_global_subprogs.c b/tools/testing/selftests/bpf/progs/verifier_global_subprogs.c
index 2250fc31574d..1e08aff7532e 100644
--- a/tools/testing/selftests/bpf/progs/verifier_global_subprogs.c
+++ b/tools/testing/selftests/bpf/progs/verifier_global_subprogs.c
@@ -357,6 +357,100 @@ int arg_tag_ctx_syscall(void *ctx)
 	return tracing_subprog_void(ctx) + tracing_subprog_u64(ctx) + tp_whatever(ctx);
 }
 
+__weak int syscall_array_bpf_for(void *ctx __arg_ctx)
+{
+	int *arr = ctx;
+	int i;
+
+	bpf_for(i, 0, 100)
+		arr[i] *= i;
+
+	return 0;
+}
+
+SEC("?syscall")
+__success __log_level(2)
+int arg_tag_ctx_syscall_bpf_for(void *ctx)
+{
+	return syscall_array_bpf_for(ctx);
+}
+
+SEC("syscall")
+__auxiliary
+int syscall_tailcall_target(void *ctx)
+{
+	return syscall_array_bpf_for(ctx);
+}
+
+struct {
+	__uint(type, BPF_MAP_TYPE_PROG_ARRAY);
+	__uint(max_entries, 1);
+	__uint(key_size, sizeof(__u32));
+	__array(values, int (void *));
+} syscall_prog_array SEC(".maps") = {
+	.values = {
+		[0] = (void *)&syscall_tailcall_target,
+	},
+};
+
+SEC("?syscall")
+__success __log_level(2)
+int arg_tag_ctx_syscall_tailcall(void *ctx)
+{
+	bpf_tail_call(ctx, &syscall_prog_array, 0);
+	return 0;
+}
+
+SEC("?syscall")
+__failure __log_level(2)
+__msg("dereference of modified ctx ptr R1 off=8 disallowed")
+int arg_tag_ctx_syscall_tailcall_fixed_off_bad(void *ctx)
+{
+	char *p = ctx;
+
+	p += 8;
+	bpf_tail_call(p, &syscall_prog_array, 0);
+	return 0;
+}
+
+SEC("?syscall")
+__failure __log_level(2)
+__msg("variable ctx access var_off=(0x0; 0x4) disallowed")
+int arg_tag_ctx_syscall_tailcall_var_off_bad(void *ctx)
+{
+	__u64 off = bpf_get_prandom_u32();
+	char *p = ctx;
+
+	off &= 4;
+	p += off;
+	bpf_tail_call(p, &syscall_prog_array, 0);
+	return 0;
+}
+
+SEC("?syscall")
+__failure __log_level(2)
+__msg("dereference of modified ctx ptr R1 off=8 disallowed")
+int arg_tag_ctx_syscall_fixed_off_bad(void *ctx)
+{
+	char *p = ctx;
+
+	p += 8;
+	return subprog_ctx_tag(p);
+}
+
+SEC("?syscall")
+__failure __log_level(2)
+__msg("variable ctx access var_off=(0x0; 0x4) disallowed")
+int arg_tag_ctx_syscall_var_off_bad(void *ctx)
+{
+	__u64 off = bpf_get_prandom_u32();
+	char *p = ctx;
+
+	off &= 4;
+	p += off;
+	return subprog_ctx_tag(p);
+}
+
 __weak int subprog_dynptr(struct bpf_dynptr *dptr)
 {
 	long *d, t, buf[1] = {};
-- 
2.52.0

Re: [PATCH bpf-next v3 0/3] Allow variable offsets for syscall PTR_TO_CTX

[Puranjay Mohan]

Kumar Kartikeya Dwivedi <memxor@gmail.com> writes:

> Enable pointer modification with variable offsets accumulated in the
> register for PTR_TO_CTX for syscall programs where it won't be
> rewritten, and the context is user-supplied and checked against the max
> offset. See patches for details. Fixed offset support landed in [0].
>
> By combining this set with [0], examples like the one below should
> succeed verification now.
>
>   SEC("syscall")
>   int prog(void *ctx) {
> 	int *arr = ctx;
> 	int i;
>
> 	bpf_for(i, 0, 100)
> 		arr[i] *= i;
>
> 	return 0;
>   }
>
>   [0]: https://lore.kernel.org/bpf/20260227005725.1247305-1-memxor@gmail.com
>
> Changelog:
> ----------
> v2 -> v3
> v2: https://lore.kernel.org/bpf/20260318075133.1031781-1-memxor@gmail.com
>
>  * Prevent arg_type for KF_ARG_PTR_TO_CTX from applying to other cases
>    due to preceding fallthrough. (Gemini/Sashiko)
>
> v1 -> v2
> v1: https://lore.kernel.org/bpf/20260317111850.2107846-2-memxor@gmail.com
>
>  * Harden check_func_arg_reg_off check with ARG_PTR_TO_CTX.
>  * Add tests for unmodified ctx into tail calls.
>  * Squash unmodified ctx change into base commit.
>  * Add Reviewed-by's from Emil.
>
> Kumar Kartikeya Dwivedi (3):
>   bpf: Support variable offsets for syscall PTR_TO_CTX
>   selftests/bpf: Add syscall ctx variable offset tests
>   selftests/bpf: Test modified syscall ctx for ARG_PTR_TO_CTX
>
>  kernel/bpf/verifier.c                         |  67 ++--
>  .../selftests/bpf/prog_tests/verifier.c       |  34 +-
>  .../selftests/bpf/progs/verifier_ctx.c        | 356 +++++++++++++++---
>  .../bpf/progs/verifier_global_subprogs.c      |  95 ++++-
>  .../selftests/bpf/test_kmods/bpf_testmod.c    |   2 +-
>  5 files changed, 462 insertions(+), 92 deletions(-)
>
>
> base-commit: 77378dabb50f593c756d393d8eacb0b91b758863
> -- 
> 2.52.0

Acked-by: Puranjay Mohan <puranjay@kernel.org>

Thanks,
Puranjay

Re: [PATCH bpf-next v3 1/3] bpf: Support variable offsets for syscall PTR_TO_CTX

[Mykyta Yatsenko]

Kumar Kartikeya Dwivedi <memxor@gmail.com> writes:

> 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.
>
> Take care to ensure that when arg_type is ARG_PTR_TO_CTX, we do not
> relax any fixed or variable offset constraints around PTR_TO_CTX even in
> syscall programs, and require them to be passed unmodified. There are
> several reasons why this is necessary. First, if we pass a modified ctx,
> then the global subprog's accesses will not update the max_ctx_offset to
> its true maximum offset, and can lead to out of bounds accesses. Second,
> tail called program (or extension program replacing global subprog) where
> their max_ctx_offset exceeds the program they are being called from can
> also cause issues. For the latter, unmodified PTR_TO_CTX is the first
> requirement for the fix, the second is ensuring max_ctx_offset >= the
> program they are being called from, which has to be a separate change
> not made in this commit.
>
> All in all, we can hint using arg_type when we expect ARG_PTR_TO_CTX and
> make our relaxation around offsets conditional on it.
>
> Cc: Tejun Heo <tj@kernel.org>
> Cc: Dan Schatzberg <dschatzberg@meta.com>
> Reviewed-by: Emil Tsalapatis <emil@etsalapatis.com>
> Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
> ---
>  kernel/bpf/verifier.c                         | 67 ++++++++++++-------
>  .../bpf/progs/verifier_global_subprogs.c      |  1 -
>  2 files changed, 43 insertions(+), 25 deletions(-)
>
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 01c18f4268de..14bf64e0c600 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,16 +7858,26 @@ 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;
> -
>  		/*
>  		 * 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 allowed to be variable, then the maximum
> +		 * possible offset is simulated (which is equal to var_off.value
> +		 * when var_off is constant).
>  		 */
> -		if (fixed_off_ok)
> -			off += reg->var_off.value;
> +		if (var_off_ok) {
> +			err = check_mem_region_access(env, regno, off, size, U16_MAX, false);
> +			if (err)
> +				return err;
> +			off += reg->umax_value;
> +		} else {
> +			err = __check_ptr_off_reg(env, reg, regno, fixed_off_ok);
> +			if (err < 0)
> +				return err;
> +			if (fixed_off_ok)
> +				off += reg->var_off.value;
> +		}
nit: this code looks a bit complex, I wonder if it makes sense to encode
the context offset mode into an enum:
enum bpf_ctx_allowed_off {
     CTX_OFF_VAR,
     CTX_OFF_FIXED,
     CTX_OFF_NONE
};

we can factor out a helper that returns allowed offset mode:
```
enum bpf_ctx_allowed_off get_context_allowed_offset(env)
{
        if (resolve_prog_type(env->prog) == BPF_PROG_TYPE_SYSCALL)
	        return CTX_OFF_VAR;
        else if (!env->ops->convert_ctx_access)
        	return CTX_OFF_FIXED;
        else
                return CTX_OFF_NONE;
}
```

The enum makes the three-way exclusive modes explicit, eliminates the
implicit priority and more self-documenting.

The enum can also be used below.

> +
>  		err = check_ctx_access(env, insn_idx, off, size, t, &info);
>  		if (err)
>  			verbose_linfo(env, insn_idx, "; ");
> @@ -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) {
Why did you remove this block here, it should correspond to fixed
offset, and is not processed in the resolve_prog_type(env->prog) ==
BPF_PROG_TYPE_SYSCALL) branch.
> -			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) {
If we introduce bpf_ctx_allowed_off enum, this check could be modified
to if (get_context_allowed_offset() == CTX_OFF_VAR) here and also in
other place as well, does it capture the logic better?
I'm not 100% sure if these use cases are worth adding a separate enum,
though, let me know what you think.
> +			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,17 @@ 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, but
> +		 * only when the argument is passed as memory, not ctx,
> +		 * otherwise we may get modified ctx in tail called programs and
> +		 * global subprogs (that may act as extension prog hooks).
> +		 */
> +		if (arg_type != ARG_PTR_TO_CTX &&
> +		    resolve_prog_type(env->prog) == BPF_PROG_TYPE_SYSCALL)
This looks like another instance where we check for prog_type==syscall,
but actually mean: is variable offset into ctx is allowed.
> +			return 0;
> +		fallthrough;
>  	default:
>  		return __check_ptr_off_reg(env, reg, regno, false);
>  	}
> @@ -10757,7 +10774,7 @@ static int btf_check_func_arg_match(struct bpf_verifier_env *env, int subprog,
>  			 * invalid memory access.
>  			 */
>  		} else if (arg->arg_type == ARG_PTR_TO_CTX) {
> -			ret = check_func_arg_reg_off(env, reg, regno, ARG_DONTCARE);
> +			ret = check_func_arg_reg_off(env, reg, regno, ARG_PTR_TO_CTX);
>  			if (ret < 0)
>  				return ret;
>  			/* If function expects ctx type in BTF check that caller
> @@ -13565,7 +13582,6 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_kfunc_call_
>  				}
>  			}
>  			fallthrough;
> -		case KF_ARG_PTR_TO_CTX:
>  		case KF_ARG_PTR_TO_DYNPTR:
>  		case KF_ARG_PTR_TO_ITER:
>  		case KF_ARG_PTR_TO_LIST_HEAD:
> @@ -13583,6 +13599,9 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_kfunc_call_
>  		case KF_ARG_PTR_TO_IRQ_FLAG:
>  		case KF_ARG_PTR_TO_RES_SPIN_LOCK:
>  			break;
> +		case KF_ARG_PTR_TO_CTX:
> +			arg_type = ARG_PTR_TO_CTX;
> +			break;
>  		default:
>  			verifier_bug(env, "unknown kfunc arg type %d", kf_arg_type);
>  			return -EFAULT;
> 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)
>  {
> -- 
> 2.52.0

Re: [PATCH bpf-next v3 2/3] selftests/bpf: Add syscall ctx variable offset tests

[Mykyta Yatsenko]

Kumar Kartikeya Dwivedi <memxor@gmail.com> writes:

> Add various tests to exercise fixed and variable offsets on PTR_TO_CTX
> for syscall programs, and cover disallowed cases for other program types
> lacking convert_ctx_access callback. Load verifier_ctx with CAP_SYS_ADMIN
> so that kfunc related logic can be tested. While at it, convert assembly
> tests to C.
>
> Reviewed-by: Emil Tsalapatis <emil@etsalapatis.com>
> Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
> ---
>  .../selftests/bpf/prog_tests/verifier.c       |  34 +-
>  .../selftests/bpf/progs/verifier_ctx.c        | 356 +++++++++++++++---
>  .../selftests/bpf/test_kmods/bpf_testmod.c    |   2 +-
>  3 files changed, 325 insertions(+), 67 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/verifier.c b/tools/testing/selftests/bpf/prog_tests/verifier.c
> index 8cdfd74c95d7..04d5f46264a3 100644
> --- a/tools/testing/selftests/bpf/prog_tests/verifier.c
> +++ b/tools/testing/selftests/bpf/prog_tests/verifier.c
> @@ -126,29 +126,35 @@ struct test_val {
>  __maybe_unused
>  static void run_tests_aux(const char *skel_name,
>  			  skel_elf_bytes_fn elf_bytes_factory,
> -			  pre_execution_cb pre_execution_cb)
> +			  pre_execution_cb pre_execution_cb,
> +			  bool drop_sys_admin)
>  {
>  	struct test_loader tester = {};
>  	__u64 old_caps;
>  	int err;
>  
> -	/* test_verifier tests are executed w/o CAP_SYS_ADMIN, do the same here */
> -	err = cap_disable_effective(1ULL << CAP_SYS_ADMIN, &old_caps);
> -	if (err) {
> -		PRINT_FAIL("failed to drop CAP_SYS_ADMIN: %i, %s\n", err, strerror(-err));
> -		return;
> +	if (drop_sys_admin) {
> +		/* test_verifier tests are executed w/o CAP_SYS_ADMIN, do the same here */
> +		err = cap_disable_effective(1ULL << CAP_SYS_ADMIN, &old_caps);
> +		if (err) {
> +			PRINT_FAIL("failed to drop CAP_SYS_ADMIN: %i, %s\n", err, strerror(-err));
> +			return;
> +		}
>  	}
>  
>  	test_loader__set_pre_execution_cb(&tester, pre_execution_cb);
>  	test_loader__run_subtests(&tester, skel_name, elf_bytes_factory);
>  	test_loader_fini(&tester);
>  
> -	err = cap_enable_effective(old_caps, NULL);
> -	if (err)
> -		PRINT_FAIL("failed to restore CAP_SYS_ADMIN: %i, %s\n", err, strerror(-err));
> +	if (drop_sys_admin) {
> +		err = cap_enable_effective(old_caps, NULL);
> +		if (err)
> +			PRINT_FAIL("failed to restore CAP_SYS_ADMIN: %i, %s\n", err, strerror(-err));
> +	}
>  }
>  
> -#define RUN(skel) run_tests_aux(#skel, skel##__elf_bytes, NULL)
> +#define RUN(skel) run_tests_aux(#skel, skel##__elf_bytes, NULL, true)
> +#define RUN_WITH_CAP_SYS_ADMIN(skel) run_tests_aux(#skel, skel##__elf_bytes, NULL, false)
>  
>  void test_verifier_align(void)                { RUN(verifier_align); }
>  void test_verifier_and(void)                  { RUN(verifier_and); }
> @@ -173,7 +179,7 @@ void test_verifier_cgroup_skb(void)           { RUN(verifier_cgroup_skb); }
>  void test_verifier_cgroup_storage(void)       { RUN(verifier_cgroup_storage); }
>  void test_verifier_const(void)                { RUN(verifier_const); }
>  void test_verifier_const_or(void)             { RUN(verifier_const_or); }
> -void test_verifier_ctx(void)                  { RUN(verifier_ctx); }
> +void test_verifier_ctx(void)                  { RUN_WITH_CAP_SYS_ADMIN(verifier_ctx); }
>  void test_verifier_ctx_sk_msg(void)           { RUN(verifier_ctx_sk_msg); }
>  void test_verifier_d_path(void)               { RUN(verifier_d_path); }
>  void test_verifier_default_trusted_ptr(void)  { RUN_TESTS(verifier_default_trusted_ptr); }
> @@ -293,7 +299,8 @@ void test_verifier_array_access(void)
>  {
>  	run_tests_aux("verifier_array_access",
>  		      verifier_array_access__elf_bytes,
> -		      init_array_access_maps);
> +		      init_array_access_maps,
> +		      true);
>  }
>  void test_verifier_async_cb_context(void)    { RUN(verifier_async_cb_context); }
>  
> @@ -306,5 +313,6 @@ void test_verifier_value_ptr_arith(void)
>  {
>  	run_tests_aux("verifier_value_ptr_arith",
>  		      verifier_value_ptr_arith__elf_bytes,
> -		      init_value_ptr_arith_maps);
> +		      init_value_ptr_arith_maps,
> +		      true);
>  }
> diff --git a/tools/testing/selftests/bpf/progs/verifier_ctx.c b/tools/testing/selftests/bpf/progs/verifier_ctx.c
> index 371780290c0d..c054c8cb7242 100644
> --- a/tools/testing/selftests/bpf/progs/verifier_ctx.c
> +++ b/tools/testing/selftests/bpf/progs/verifier_ctx.c
> @@ -4,6 +4,10 @@
>  #include "vmlinux.h"
>  #include <bpf/bpf_helpers.h>
>  #include "bpf_misc.h"
> +#include "../test_kmods/bpf_testmod_kfunc.h"
> +
> +const char ctx_strncmp_target[] = "ctx";
> +const char ctx_snprintf_fmt[] = "";
>  
>  SEC("tc")
>  __description("context stores via BPF_ATOMIC")
> @@ -69,7 +73,6 @@ __naked void ctx_pointer_to_helper_1(void)
>  SEC("socket")
>  __description("pass modified ctx pointer to helper, 2")
>  __failure __msg("negative offset ctx ptr R1 off=-612 disallowed")
> -__failure_unpriv __msg_unpriv("negative offset ctx ptr R1 off=-612 disallowed")
>  __naked void ctx_pointer_to_helper_2(void)
>  {
>  	asm volatile ("					\
> @@ -295,77 +298,324 @@ padding_access("sk_reuseport", sk_reuseport_md, hash, 4);
>  SEC("syscall")
>  __description("syscall: write to ctx with fixed offset")
>  __success
> -__naked void syscall_ctx_fixed_off_write(void)
> +int syscall_ctx_fixed_off_write(void *ctx)
>  {
> -	asm volatile ("					\
> -	r0 = 0;						\
> -	*(u32*)(r1 + 0) = r0;				\
> -	r1 += 4;					\
> -	*(u32*)(r1 + 0) = r0;				\
> -	exit;						\
> -"	::: __clobber_all);
> +	char *p = ctx;
> +
> +	*(__u32 *)p = 0;
> +	*(__u32 *)(p + 4) = 0;
> +	return 0;
> +}
> +
> +SEC("syscall")
> +__description("syscall: read ctx with fixed offset")
> +__success
> +int syscall_ctx_fixed_off_read(void *ctx)
> +{
> +	char *p = ctx;
> +	volatile __u32 val;
> +
> +	val = *(__u32 *)(p + 4);
> +	(void)val;
> +	return 0;
> +}
> +
> +SEC("syscall")
> +__description("syscall: read ctx with variable offset")
> +__success
> +int syscall_ctx_var_off_read(void *ctx)
> +{
> +	__u64 off = bpf_get_prandom_u32();
> +	char *p = ctx;
> +	volatile __u32 val;
> +
> +	off &= 0xfc;
> +	p += off;
> +	val = *(__u32 *)p;
> +	(void)val;
> +	return 0;
> +}
> +
> +SEC("syscall")
> +__description("syscall: write ctx with variable offset")
> +__success
> +int syscall_ctx_var_off_write(void *ctx)
> +{
> +	__u64 off = bpf_get_prandom_u32();
> +	char *p = ctx;
> +
> +	off &= 0xfc;
> +	p += off;
> +	*(__u32 *)p = 0;
> +	return 0;
> +}
> +
> +SEC("syscall")
> +__description("syscall: reject negative variable offset ctx access")
> +__failure __msg("min value is negative")
> +int syscall_ctx_neg_var_off(void *ctx)
> +{
> +	__u64 off = bpf_get_prandom_u32();
> +	char *p = ctx;
> +
> +	off &= 4;
> +	p -= off;
> +	return *(__u32 *)p;
> +}
> +
> +SEC("syscall")
> +__description("syscall: reject unbounded variable offset ctx access")
> +__failure __msg("unbounded memory access")
> +int syscall_ctx_unbounded_var_off(void *ctx)
> +{
> +	__u64 off = (__u32)bpf_get_prandom_u32();
> +	char *p = ctx;
> +
> +	off <<= 2;
> +	p += off;
> +	return *(__u32 *)p;
> +}
> +
> +SEC("syscall")
> +__description("syscall: helper read ctx with fixed offset")
> +__success
> +int syscall_ctx_helper_fixed_off_read(void *ctx)
> +{
> +	char *p = ctx;
> +
> +	p += 4;
> +	return bpf_strncmp(p, 4, ctx_strncmp_target);
> +}
> +
> +SEC("syscall")
> +__description("syscall: helper write ctx with fixed offset")
> +__success
> +int syscall_ctx_helper_fixed_off_write(void *ctx)
> +{
> +	char *p = ctx;
> +
> +	p += 4;
> +	return bpf_probe_read_kernel(p, 4, 0);
> +}
> +
> +SEC("syscall")
> +__description("syscall: helper read ctx with variable offset")
> +__success
> +int syscall_ctx_helper_var_off_read(void *ctx)
> +{
> +	__u64 off = bpf_get_prandom_u32();
> +	char *p = ctx;
> +
> +	off &= 0xfc;
> +	p += off;
> +	return bpf_strncmp(p, 4, ctx_strncmp_target);
> +}
> +
> +SEC("syscall")
> +__description("syscall: helper write ctx with variable offset")
> +__success
> +int syscall_ctx_helper_var_off_write(void *ctx)
> +{
> +	__u64 off = bpf_get_prandom_u32();
> +	char *p = ctx;
> +
> +	off &= 0xfc;
> +	p += off;
> +	return bpf_probe_read_kernel(p, 4, 0);
> +}
> +
> +SEC("syscall")
> +__description("syscall: helper read zero-sized ctx access")
> +__success
> +int syscall_ctx_helper_zero_sized_read(void *ctx)
> +{
> +	return bpf_snprintf(0, 0, ctx_snprintf_fmt, ctx, 0);
> +}
> +
> +SEC("syscall")
> +__description("syscall: helper write zero-sized ctx access")
> +__success
> +int syscall_ctx_helper_zero_sized_write(void *ctx)
> +{
> +	return bpf_probe_read_kernel(ctx, 0, 0);
> +}
> +
> +SEC("syscall")
> +__description("syscall: kfunc access ctx with fixed offset")
> +__success
> +int syscall_ctx_kfunc_fixed_off(void *ctx)
> +{
> +	char *p = ctx;
> +
> +	p += 4;
> +	bpf_kfunc_call_test_mem_len_pass1(p, 4);
> +	return 0;
> +}
> +
> +SEC("syscall")
> +__description("syscall: kfunc access ctx with variable offset")
> +__success
> +int syscall_ctx_kfunc_var_off(void *ctx)
> +{
> +	__u64 off = bpf_get_prandom_u32();
> +	char *p = ctx;
> +
> +	off &= 0xfc;
> +	p += off;
> +	bpf_kfunc_call_test_mem_len_pass1(p, 4);
> +	return 0;
> +}
> +
> +SEC("syscall")
> +__description("syscall: kfunc access zero-sized ctx")
> +__success
> +int syscall_ctx_kfunc_zero_sized(void *ctx)
> +{
> +	bpf_kfunc_call_test_mem_len_pass1(ctx, 0);
> +	return 0;
>  }
Tests are comprehensive. Do we need to test rejection of unaligned read
too?
Acked-by: Mykyta Yatsenko <yatsenko@meta.com>
>  
>  /*
> - * Test that program types without convert_ctx_access can dereference
> - * their ctx pointer after adding a fixed offset. Variable and negative
> - * offsets should still be rejected.
> + * For non-syscall program types without convert_ctx_access, direct ctx
> + * dereference is still allowed after adding a fixed offset, while variable
> + * and negative direct accesses reject.
> + *
> + * Passing ctx as a helper or kfunc memory argument is only permitted for
> + * syscall programs, so the helper and kfunc cases below validate rejection
> + * for non-syscall ctx pointers at fixed, variable, and zero-sized accesses.
>   */
> -#define no_rewrite_ctx_access(type, name, off, ld_op)			\
> +#define no_rewrite_ctx_access(type, name, off, load_t)			\
>  	SEC(type)							\
>  	__description(type ": read ctx at fixed offset")		\
>  	__success							\
> -	__naked void no_rewrite_##name##_fixed(void)			\
> +	int no_rewrite_##name##_fixed(void *ctx)			\
>  	{								\
> -		asm volatile ("						\
> -		r1 += %[__off];						\
> -		r0 = *(" #ld_op " *)(r1 + 0);				\
> -		r0 = 0;							\
> -		exit;"							\
> -		:							\
> -		: __imm_const(__off, off)				\
> -		: __clobber_all);					\
> +		char *p = ctx;						\
> +		volatile load_t val;					\
> +									\
> +		val = *(load_t *)(p + off);				\
> +		(void)val;						\
> +		return 0;						\
>  	}								\
>  	SEC(type)							\
>  	__description(type ": reject variable offset ctx access")	\
>  	__failure __msg("variable ctx access var_off=")			\
> -	__naked void no_rewrite_##name##_var(void)			\
> +	int no_rewrite_##name##_var(void *ctx)			\
>  	{								\
> -		asm volatile ("						\
> -		r6 = r1;						\
> -		call %[bpf_get_prandom_u32];				\
> -		r1 = r6;						\
> -		r0 &= 4;						\
> -		r1 += r0;						\
> -		r0 = *(" #ld_op " *)(r1 + 0);				\
> -		r0 = 0;							\
> -		exit;"							\
> -		:							\
> -		: __imm(bpf_get_prandom_u32)				\
> -		: __clobber_all);					\
> +		__u64 off_var = bpf_get_prandom_u32();			\
> +		char *p = ctx;						\
> +									\
> +		off_var &= 4;						\
> +		p += off_var;						\
> +		return *(load_t *)p;					\
>  	}								\
>  	SEC(type)							\
>  	__description(type ": reject negative offset ctx access")	\
> -	__failure __msg("negative offset ctx ptr")			\
> -	__naked void no_rewrite_##name##_neg(void)			\
> +	__failure __msg("invalid bpf_context access")			\
> +	int no_rewrite_##name##_neg(void *ctx)			\
>  	{								\
> -		asm volatile ("						\
> -		r1 += %[__neg_off];					\
> -		r0 = *(" #ld_op " *)(r1 + 0);				\
> -		r0 = 0;							\
> -		exit;"							\
> -		:							\
> -		: __imm_const(__neg_off, -(off))			\
> -		: __clobber_all);					\
> +		char *p = ctx;						\
> +									\
> +		p -= 612;						\
> +		return *(load_t *)p;					\
> +	}								\
> +	SEC(type)							\
> +	__description(type ": reject helper read ctx at fixed offset")	\
> +	__failure __msg("dereference of modified ctx ptr")		\
> +	int no_rewrite_##name##_helper_read_fixed(void *ctx)		\
> +	{								\
> +		char *p = ctx;						\
> +									\
> +		p += off;						\
> +		return bpf_strncmp(p, 4, ctx_strncmp_target);		\
> +	}								\
> +	SEC(type)							\
> +	__description(type ": reject helper write ctx at fixed offset")	\
> +	__failure __msg("dereference of modified ctx ptr")		\
> +	int no_rewrite_##name##_helper_write_fixed(void *ctx)		\
> +	{								\
> +		char *p = ctx;						\
> +									\
> +		p += off;						\
> +		return bpf_probe_read_kernel(p, 4, 0);			\
> +	}								\
> +	SEC(type)							\
> +	__description(type ": reject helper read ctx with variable offset") \
> +	__failure __msg("variable ctx access var_off=")			\
> +	int no_rewrite_##name##_helper_read_var(void *ctx)		\
> +	{								\
> +		__u64 off_var = bpf_get_prandom_u32();			\
> +		char *p = ctx;						\
> +									\
> +		off_var &= 4;						\
> +		p += off_var;						\
> +		return bpf_strncmp(p, 4, ctx_strncmp_target);		\
> +	}								\
> +	SEC(type)							\
> +	__description(type ": reject helper write ctx with variable offset") \
> +	__failure __msg("variable ctx access var_off=")			\
> +	int no_rewrite_##name##_helper_write_var(void *ctx)		\
> +	{								\
> +		__u64 off_var = bpf_get_prandom_u32();			\
> +		char *p = ctx;						\
> +									\
> +		off_var &= 4;						\
> +		p += off_var;						\
> +		return bpf_probe_read_kernel(p, 4, 0);			\
> +	}								\
> +	SEC(type)							\
> +	__description(type ": reject helper read zero-sized ctx access") \
> +	__failure __msg("R4 type=ctx expected=fp")			\
> +	int no_rewrite_##name##_helper_read_zero(void *ctx)		\
> +	{								\
> +		return bpf_snprintf(0, 0, ctx_snprintf_fmt, ctx, 0);	\
> +	}								\
> +	SEC(type)							\
> +	__description(type ": reject helper write zero-sized ctx access") \
> +	__failure __msg("R1 type=ctx expected=fp")			\
> +	int no_rewrite_##name##_helper_write_zero(void *ctx)		\
> +	{								\
> +		return bpf_probe_read_kernel(ctx, 0, 0);			\
> +	}								\
> +	SEC(type)							\
> +	__description(type ": reject kfunc ctx at fixed offset")	\
> +	__failure __msg("dereference of modified ctx ptr")		\
> +	int no_rewrite_##name##_kfunc_fixed(void *ctx)		\
> +	{								\
> +		char *p = ctx;						\
> +									\
> +		p += off;						\
> +		bpf_kfunc_call_test_mem_len_pass1(p, 4);		\
> +		return 0;						\
> +	}								\
> +	SEC(type)							\
> +	__description(type ": reject kfunc ctx with variable offset")	\
> +	__failure __msg("variable ctx access var_off=")			\
> +	int no_rewrite_##name##_kfunc_var(void *ctx)			\
> +	{								\
> +		__u64 off_var = bpf_get_prandom_u32();			\
> +		char *p = ctx;						\
> +									\
> +		off_var &= 4;						\
> +		p += off_var;						\
> +		bpf_kfunc_call_test_mem_len_pass1(p, 4);		\
> +		return 0;						\
> +	}								\
> +	SEC(type)							\
> +	__description(type ": reject kfunc zero-sized ctx access")	\
> +	__failure __msg("R1 type=ctx expected=fp")			\
> +	int no_rewrite_##name##_kfunc_zero(void *ctx)			\
> +	{								\
> +		bpf_kfunc_call_test_mem_len_pass1(ctx, 0);		\
> +		return 0;						\
>  	}
>  
> -no_rewrite_ctx_access("syscall", syscall, 4, u32);
> -no_rewrite_ctx_access("kprobe", kprobe, 8, u64);
> -no_rewrite_ctx_access("tracepoint", tp, 8, u64);
> -no_rewrite_ctx_access("raw_tp", raw_tp, 8, u64);
> -no_rewrite_ctx_access("raw_tracepoint.w", raw_tp_w, 8, u64);
> -no_rewrite_ctx_access("fentry/bpf_modify_return_test", fentry, 8, u64);
> -no_rewrite_ctx_access("cgroup/dev", cgroup_dev, 4, u32);
> -no_rewrite_ctx_access("netfilter", netfilter, offsetof(struct bpf_nf_ctx, skb), u64);
> +no_rewrite_ctx_access("kprobe", kprobe, 8, __u64);
> +no_rewrite_ctx_access("tracepoint", tp, 8, __u64);
> +no_rewrite_ctx_access("raw_tp", raw_tp, 8, __u64);
> +no_rewrite_ctx_access("raw_tracepoint.w", raw_tp_w, 8, __u64);
> +no_rewrite_ctx_access("fentry/bpf_modify_return_test", fentry, 8, __u64);
> +no_rewrite_ctx_access("cgroup/dev", cgroup_dev, 4, __u32);
> +no_rewrite_ctx_access("netfilter", netfilter, offsetof(struct bpf_nf_ctx, skb), __u64);
>  
>  char _license[] SEC("license") = "GPL";
> diff --git a/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c b/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c
> index 94edbd2afa67..f91b484d80f2 100644
> --- a/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c
> +++ b/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c
> @@ -716,6 +716,7 @@ BTF_ID_FLAGS(func, bpf_iter_testmod_seq_next, KF_ITER_NEXT | KF_RET_NULL)
>  BTF_ID_FLAGS(func, bpf_iter_testmod_seq_destroy, KF_ITER_DESTROY)
>  BTF_ID_FLAGS(func, bpf_iter_testmod_seq_value)
>  BTF_ID_FLAGS(func, bpf_kfunc_common_test)
> +BTF_ID_FLAGS(func, bpf_kfunc_call_test_mem_len_pass1)
>  BTF_ID_FLAGS(func, bpf_kfunc_dynptr_test)
>  BTF_ID_FLAGS(func, bpf_kfunc_nested_acquire_nonzero_offset_test, KF_ACQUIRE)
>  BTF_ID_FLAGS(func, bpf_kfunc_nested_acquire_zero_offset_test, KF_ACQUIRE)
> @@ -1280,7 +1281,6 @@ BTF_ID_FLAGS(func, bpf_kfunc_call_test2)
>  BTF_ID_FLAGS(func, bpf_kfunc_call_test3)
>  BTF_ID_FLAGS(func, bpf_kfunc_call_test4)
>  BTF_ID_FLAGS(func, bpf_kfunc_call_test5)
> -BTF_ID_FLAGS(func, bpf_kfunc_call_test_mem_len_pass1)
>  BTF_ID_FLAGS(func, bpf_kfunc_call_test_mem_len_fail1)
>  BTF_ID_FLAGS(func, bpf_kfunc_call_test_mem_len_fail2)
>  BTF_ID_FLAGS(func, bpf_kfunc_call_test_acquire, KF_ACQUIRE | KF_RET_NULL)
> -- 
> 2.52.0

Re: [PATCH bpf-next v3 3/3] selftests/bpf: Test modified syscall ctx for ARG_PTR_TO_CTX

[Mykyta Yatsenko]

Kumar Kartikeya Dwivedi <memxor@gmail.com> writes:

> Ensure that global subprogs and tail calls can only accept an unmodified
> PTR_TO_CTX for syscall programs. For all other program types, fixed or
> variable offsets on PTR_TO_CTX is rejected when passed into an argument
> of any call instruction type, through the unified logic of
> check_func_arg_reg_off.
>
> Finally, add a positive example of a case that should succeed with all
> our previous changes.
>
> Reviewed-by: Emil Tsalapatis <emil@etsalapatis.com>
> Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
> ---
>  .../bpf/progs/verifier_global_subprogs.c      | 94 +++++++++++++++++++
>  1 file changed, 94 insertions(+)
>
> diff --git a/tools/testing/selftests/bpf/progs/verifier_global_subprogs.c b/tools/testing/selftests/bpf/progs/verifier_global_subprogs.c
> index 2250fc31574d..1e08aff7532e 100644
> --- a/tools/testing/selftests/bpf/progs/verifier_global_subprogs.c
> +++ b/tools/testing/selftests/bpf/progs/verifier_global_subprogs.c
> @@ -357,6 +357,100 @@ int arg_tag_ctx_syscall(void *ctx)
>  	return tracing_subprog_void(ctx) + tracing_subprog_u64(ctx) + tp_whatever(ctx);
>  }
>  
> +__weak int syscall_array_bpf_for(void *ctx __arg_ctx)
> +{
> +	int *arr = ctx;
> +	int i;
> +
> +	bpf_for(i, 0, 100)
> +		arr[i] *= i;
> +
> +	return 0;
> +}
> +
> +SEC("?syscall")
> +__success __log_level(2)
> +int arg_tag_ctx_syscall_bpf_for(void *ctx)
> +{
> +	return syscall_array_bpf_for(ctx);
> +}
> +
> +SEC("syscall")
> +__auxiliary
> +int syscall_tailcall_target(void *ctx)
> +{
> +	return syscall_array_bpf_for(ctx);
> +}
> +
> +struct {
> +	__uint(type, BPF_MAP_TYPE_PROG_ARRAY);
> +	__uint(max_entries, 1);
> +	__uint(key_size, sizeof(__u32));
> +	__array(values, int (void *));
> +} syscall_prog_array SEC(".maps") = {
> +	.values = {
> +		[0] = (void *)&syscall_tailcall_target,
> +	},
> +};
> +
> +SEC("?syscall")
> +__success __log_level(2)
> +int arg_tag_ctx_syscall_tailcall(void *ctx)
> +{
> +	bpf_tail_call(ctx, &syscall_prog_array, 0);
> +	return 0;
> +}
> +
> +SEC("?syscall")
> +__failure __log_level(2)
> +__msg("dereference of modified ctx ptr R1 off=8 disallowed")
> +int arg_tag_ctx_syscall_tailcall_fixed_off_bad(void *ctx)
> +{
> +	char *p = ctx;
> +
> +	p += 8;
> +	bpf_tail_call(p, &syscall_prog_array, 0);
> +	return 0;
> +}
> +
> +SEC("?syscall")
> +__failure __log_level(2)
> +__msg("variable ctx access var_off=(0x0; 0x4) disallowed")
> +int arg_tag_ctx_syscall_tailcall_var_off_bad(void *ctx)
> +{
> +	__u64 off = bpf_get_prandom_u32();
> +	char *p = ctx;
> +
> +	off &= 4;
> +	p += off;
> +	bpf_tail_call(p, &syscall_prog_array, 0);
> +	return 0;
> +}
> +
> +SEC("?syscall")
> +__failure __log_level(2)
> +__msg("dereference of modified ctx ptr R1 off=8 disallowed")
> +int arg_tag_ctx_syscall_fixed_off_bad(void *ctx)
> +{
> +	char *p = ctx;
> +
> +	p += 8;
> +	return subprog_ctx_tag(p);
> +}
> +
> +SEC("?syscall")
> +__failure __log_level(2)
> +__msg("variable ctx access var_off=(0x0; 0x4) disallowed")
> +int arg_tag_ctx_syscall_var_off_bad(void *ctx)
Test cases for subprog_ctx_tag() and bpf_tail_call() are duplicated,
even verifier error messages are the same. I don't see a better way to
avoid it though (macros as in prev patch look worse).
Acked-by: Mykyta Yatsenko <yatsenko@meta.com>
> +{
> +	__u64 off = bpf_get_prandom_u32();
> +	char *p = ctx;
> +
> +	off &= 4;
> +	p += off;
> +	return subprog_ctx_tag(p);
> +}
> +
>  __weak int subprog_dynptr(struct bpf_dynptr *dptr)
>  {
>  	long *d, t, buf[1] = {};
> -- 
> 2.52.0

Re: [PATCH bpf-next v3 1/3] bpf: Support variable offsets for syscall PTR_TO_CTX

[Eduard Zingerman]

On Wed, 2026-03-18 at 03:35 -0700, Kumar Kartikeya Dwivedi wrote:

[...]

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 01c18f4268de..14bf64e0c600 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,16 +7858,26 @@ 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;
> -
>  		/*
>  		 * 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 allowed to be variable, then the maximum
> +		 * possible offset is simulated (which is equal to var_off.value
> +		 * when var_off is constant).
>  		 */

Nit: I'm not sure this comment adds much value, I'd drop it altogether.

> -		if (fixed_off_ok)
> -			off += reg->var_off.value;
> +		if (var_off_ok) {
> +			err = check_mem_region_access(env, regno, off, size, U16_MAX, false);
                                                                             ^^^^^^^^
                 Nit: Maybe leave a comment here saying that the following check_ctx_access()
                      ensures proper maximal bound?
                 Nit: check_mem_region_access() which calls __check_mem_access(), maybe add
                     `case PTR_TO_CTX` there for error reporting?

> +			if (err)
> +				return err;
> +			off += reg->umax_value;
> +		} else {
> +			err = __check_ptr_off_reg(env, reg, regno, fixed_off_ok);
> +			if (err < 0)
> +				return err;
> +			if (fixed_off_ok)
> +				off += reg->var_off.value;
> +		}

Let's simplify this as:

		if (var_off_ok)
			err = check_mem_region_access(env, regno, off, size, U16_MAX, false);
		else
			err = __check_ptr_off_reg(env, reg, regno, fixed_off_ok);
		if (err < 0)
			return err;
		off += reg->umax_value;

Tbh, the above looks a bit ad-hoc, one might argue that a better way in
terms of abstraction would be to adjust bpf_verifier_ops->is_valid_access()
to accept or reject varying offset access.

>  		err = check_ctx_access(env, insn_idx, off, size, t, &info);
>  		if (err)
>  			verbose_linfo(env, insn_idx, "; ");
> @@ -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) {

Nit: this is a second place where this check is hard-coded.

> +			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;

Why is this change necessary at all? Wouldn't old code handle all
relevant cases, now that check_mem_access() is updated?

>  		}
> -
>  		fallthrough;
>  	default: /* scalar_value or invalid ptr */
>  		/* Allow zero-byte read from NULL, regardless of pointer type */

[...]

> 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")

Nit: should this test case be forked as tracepoint and non-tracepoint versions?

>  __msg("Caller passes invalid args into func#1 ('subprog_user_anon_mem')")
>  int anon_user_mem_invalid(void *ctx)
>  {

Re: [PATCH bpf-next v3 2/3] selftests/bpf: Add syscall ctx variable offset tests

[Eduard Zingerman]

On Wed, 2026-03-18 at 14:45 +0000, Mykyta Yatsenko wrote:
> Kumar Kartikeya Dwivedi <memxor@gmail.com> writes:
> 
> > Add various tests to exercise fixed and variable offsets on PTR_TO_CTX
> > for syscall programs, and cover disallowed cases for other program types
> > lacking convert_ctx_access callback. Load verifier_ctx with CAP_SYS_ADMIN
> > so that kfunc related logic can be tested. While at it, convert assembly
> > tests to C.

Nit: I'd move the conversion to a separate patch in the set.

[...]

> > @@ -173,7 +179,7 @@ void test_verifier_cgroup_skb(void)           { RUN(verifier_cgroup_skb); }
> >  void test_verifier_cgroup_storage(void)       { RUN(verifier_cgroup_storage); }
> >  void test_verifier_const(void)                { RUN(verifier_const); }
> >  void test_verifier_const_or(void)             { RUN(verifier_const_or); }
> > -void test_verifier_ctx(void)                  { RUN(verifier_ctx); }
> > +void test_verifier_ctx(void)                  { RUN_WITH_CAP_SYS_ADMIN(verifier_ctx); }

Wouldn't this be the same as to use RUN_TESTS(), like a few lines below?
Also, tjos changes the semantics by always running verifier_ctx tests
with CAP_SYS_ADMIN, is that important?

> >  void test_verifier_ctx_sk_msg(void)           { RUN(verifier_ctx_sk_msg); }
> >  void test_verifier_d_path(void)               { RUN(verifier_d_path); }
> >  void test_verifier_default_trusted_ptr(void)  { RUN_TESTS(verifier_default_trusted_ptr); }

[...]

> > diff --git a/tools/testing/selftests/bpf/progs/verifier_ctx.c
> > b/tools/testing/selftests/bpf/progs/verifier_ctx.c

[...]

> > +	SEC(type)							\
> > +	__description(type ": reject kfunc zero-sized ctx access")	\
> > +	__failure __msg("R1 type=ctx expected=fp")			\
> > +	int no_rewrite_##name##_kfunc_zero(void *ctx)			\
> > +	{								\
> > +		bpf_kfunc_call_test_mem_len_pass1(ctx, 0);		\
> > +		return 0;						\
> >  	}
> >  
> > -no_rewrite_ctx_access("syscall", syscall, 4, u32);

--- 8< ----------------------------------------------

> > -no_rewrite_ctx_access("kprobe", kprobe, 8, u64);
> > -no_rewrite_ctx_access("tracepoint", tp, 8, u64);
> > -no_rewrite_ctx_access("raw_tp", raw_tp, 8, u64);
> > -no_rewrite_ctx_access("raw_tracepoint.w", raw_tp_w, 8, u64);
> > -no_rewrite_ctx_access("fentry/bpf_modify_return_test", fentry, 8, u64);
> > -no_rewrite_ctx_access("cgroup/dev", cgroup_dev, 4, u32);
> > -no_rewrite_ctx_access("netfilter", netfilter, offsetof(struct bpf_nf_ctx, skb), u64);
> > +no_rewrite_ctx_access("kprobe", kprobe, 8, __u64);
> > +no_rewrite_ctx_access("tracepoint", tp, 8, __u64);
> > +no_rewrite_ctx_access("raw_tp", raw_tp, 8, __u64);
> > +no_rewrite_ctx_access("raw_tracepoint.w", raw_tp_w, 8, __u64);
> > +no_rewrite_ctx_access("fentry/bpf_modify_return_test", fentry, 8, __u64);
> > +no_rewrite_ctx_access("cgroup/dev", cgroup_dev, 4, __u32);
> > +no_rewrite_ctx_access("netfilter", netfilter, offsetof(struct bpf_nf_ctx, skb), __u64);

---------------------------------------------- >8 ---

The chunks above differ only in '__' prefix for u32/u64.

[...]

Re: [PATCH bpf-next v3 1/3] bpf: Support variable offsets for syscall PTR_TO_CTX

[Kumar Kartikeya Dwivedi]

On Wed, 18 Mar 2026 at 22:13, Eduard Zingerman <eddyz87@gmail.com> wrote:
>
> On Wed, 2026-03-18 at 03:35 -0700, Kumar Kartikeya Dwivedi wrote:
>
> [...]
>
> > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> > index 01c18f4268de..14bf64e0c600 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,16 +7858,26 @@ 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;
> > -
> >               /*
> >                * 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 allowed to be variable, then the maximum
> > +              * possible offset is simulated (which is equal to var_off.value
> > +              * when var_off is constant).
> >                */
>
> Nit: I'm not sure this comment adds much value, I'd drop it altogether.
>

Ok.

> > -             if (fixed_off_ok)
> > -                     off += reg->var_off.value;
> > +             if (var_off_ok) {
> > +                     err = check_mem_region_access(env, regno, off, size, U16_MAX, false);
>                                                                              ^^^^^^^^
>                  Nit: Maybe leave a comment here saying that the following check_ctx_access()
>                       ensures proper maximal bound?
>                  Nit: check_mem_region_access() which calls __check_mem_access(), maybe add
>                      `case PTR_TO_CTX` there for error reporting?

Ack.

>
> > +                     if (err)
> > +                             return err;
> > +                     off += reg->umax_value;
> > +             } else {
> > +                     err = __check_ptr_off_reg(env, reg, regno, fixed_off_ok);
> > +                     if (err < 0)
> > +                             return err;
> > +                     if (fixed_off_ok)
> > +                             off += reg->var_off.value;
> > +             }
>
> Let's simplify this as:
>
>                 if (var_off_ok)
>                         err = check_mem_region_access(env, regno, off, size, U16_MAX, false);
>                 else
>                         err = __check_ptr_off_reg(env, reg, regno, fixed_off_ok);
>                 if (err < 0)
>                         return err;
>                 off += reg->umax_value;

Ack.

>
> Tbh, the above looks a bit ad-hoc, one might argue that a better way in
> terms of abstraction would be to adjust bpf_verifier_ops->is_valid_access()
> to accept or reject varying offset access.

I think for all current cases, the callback is supplied to make sure
the fixed offset is well formed.
There isn't really "valid or invalid" access otherwise, esp. for
syscall programs.
We can probably also drop this from syscall_prog_is_valid_access.
> if (off % size != 0)
Unaligned accesses should be fine.

If we moved it inside we'd have to repeat this ptr offset thing for
most of them except syscall.
Second, it does not have access to either env or reg to do the
check_mem_region_access.

>
> >               err = check_ctx_access(env, insn_idx, off, size, t, &info);
> >               if (err)
> >                       verbose_linfo(env, insn_idx, "; ");
> > @@ -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) {
>
> Nit: this is a second place where this check is hard-coded.

Any better ideas? This program type is sort of special wrt semantics of ctx.
Unlike all others, ctx here is user supplied, and rw, and carries no
kernel data.

>
> > +                     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;
>
> Why is this change necessary at all? Wouldn't old code handle all
> relevant cases, now that check_mem_access() is updated?

I think we cannot pass the access_size through, check_mem_access is
meant to simulate an instruction access.
Earlier it was doing access on the final byte since pointer
modification was not permitted, so real register offset was always
zero.
All we needed to check was whether access_size - 1 fell within bounds,
more importantly just record the max_ctx_offset since there's no
bounds except U16_MAX.
Now, the register is modified, and we can access lengths wider than
those we get from bpf_size_to_bytes.

>
> >               }
> > -
> >               fallthrough;
> >       default: /* scalar_value or invalid ptr */
> >               /* Allow zero-byte read from NULL, regardless of pointer type */
>
> [...]
>
> > 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")
>
> Nit: should this test case be forked as tracepoint and non-tracepoint versions?

In terms of coverage? I have those in the next patch already.

>
> >  __msg("Caller passes invalid args into func#1 ('subprog_user_anon_mem')")
> >  int anon_user_mem_invalid(void *ctx)
> >  {

Re: [PATCH bpf-next v3 2/3] selftests/bpf: Add syscall ctx variable offset tests

[Kumar Kartikeya Dwivedi]

On Wed, 18 Mar 2026 at 22:41, Eduard Zingerman <eddyz87@gmail.com> wrote:
>
> On Wed, 2026-03-18 at 14:45 +0000, Mykyta Yatsenko wrote:
> > Kumar Kartikeya Dwivedi <memxor@gmail.com> writes:
> >
> > > Add various tests to exercise fixed and variable offsets on PTR_TO_CTX
> > > for syscall programs, and cover disallowed cases for other program types
> > > lacking convert_ctx_access callback. Load verifier_ctx with CAP_SYS_ADMIN
> > > so that kfunc related logic can be tested. While at it, convert assembly
> > > tests to C.
>
> Nit: I'd move the conversion to a separate patch in the set.
>
> [...]
>
> > > @@ -173,7 +179,7 @@ void test_verifier_cgroup_skb(void)           { RUN(verifier_cgroup_skb); }
> > >  void test_verifier_cgroup_storage(void)       { RUN(verifier_cgroup_storage); }
> > >  void test_verifier_const(void)                { RUN(verifier_const); }
> > >  void test_verifier_const_or(void)             { RUN(verifier_const_or); }
> > > -void test_verifier_ctx(void)                  { RUN(verifier_ctx); }
> > > +void test_verifier_ctx(void)                  { RUN_WITH_CAP_SYS_ADMIN(verifier_ctx); }
>
> Wouldn't this be the same as to use RUN_TESTS(), like a few lines below?

Oh yeah, let me try that.

> Also, tjos changes the semantics by always running verifier_ctx tests
> with CAP_SYS_ADMIN, is that important?
>

If RUN_TESTS works I will drop it, yes, but I think it mostly matters
for priv/unpriv testing.

> > >  void test_verifier_ctx_sk_msg(void)           { RUN(verifier_ctx_sk_msg); }
> > >  void test_verifier_d_path(void)               { RUN(verifier_d_path); }
> > >  void test_verifier_default_trusted_ptr(void)  { RUN_TESTS(verifier_default_trusted_ptr); }
>
> [...]
>
> > > diff --git a/tools/testing/selftests/bpf/progs/verifier_ctx.c
> > > b/tools/testing/selftests/bpf/progs/verifier_ctx.c
>
> [...]
>
> > > +   SEC(type)                                                       \
> > > +   __description(type ": reject kfunc zero-sized ctx access")      \
> > > +   __failure __msg("R1 type=ctx expected=fp")                      \
> > > +   int no_rewrite_##name##_kfunc_zero(void *ctx)                   \
> > > +   {                                                               \
> > > +           bpf_kfunc_call_test_mem_len_pass1(ctx, 0);              \
> > > +           return 0;                                               \
> > >     }
> > >
> > > -no_rewrite_ctx_access("syscall", syscall, 4, u32);
>
> --- 8< ----------------------------------------------
>
> > > -no_rewrite_ctx_access("kprobe", kprobe, 8, u64);
> > > -no_rewrite_ctx_access("tracepoint", tp, 8, u64);
> > > -no_rewrite_ctx_access("raw_tp", raw_tp, 8, u64);
> > > -no_rewrite_ctx_access("raw_tracepoint.w", raw_tp_w, 8, u64);
> > > -no_rewrite_ctx_access("fentry/bpf_modify_return_test", fentry, 8, u64);
> > > -no_rewrite_ctx_access("cgroup/dev", cgroup_dev, 4, u32);
> > > -no_rewrite_ctx_access("netfilter", netfilter, offsetof(struct bpf_nf_ctx, skb), u64);
> > > +no_rewrite_ctx_access("kprobe", kprobe, 8, __u64);
> > > +no_rewrite_ctx_access("tracepoint", tp, 8, __u64);
> > > +no_rewrite_ctx_access("raw_tp", raw_tp, 8, __u64);
> > > +no_rewrite_ctx_access("raw_tracepoint.w", raw_tp_w, 8, __u64);
> > > +no_rewrite_ctx_access("fentry/bpf_modify_return_test", fentry, 8, __u64);
> > > +no_rewrite_ctx_access("cgroup/dev", cgroup_dev, 4, __u32);
> > > +no_rewrite_ctx_access("netfilter", netfilter, offsetof(struct bpf_nf_ctx, skb), __u64);
>
> ---------------------------------------------- >8 ---
>
> The chunks above differ only in '__' prefix for u32/u64.
>

Ack, will clean up.

> [...]

Re: [PATCH bpf-next v3 1/3] bpf: Support variable offsets for syscall PTR_TO_CTX

[Kumar Kartikeya Dwivedi]

On Wed, 18 Mar 2026 at 15:06, Mykyta Yatsenko
<mykyta.yatsenko5@gmail.com> wrote:
>
> Kumar Kartikeya Dwivedi <memxor@gmail.com> writes:
>
> > 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.
> >
> > Take care to ensure that when arg_type is ARG_PTR_TO_CTX, we do not
> > relax any fixed or variable offset constraints around PTR_TO_CTX even in
> > syscall programs, and require them to be passed unmodified. There are
> > several reasons why this is necessary. First, if we pass a modified ctx,
> > then the global subprog's accesses will not update the max_ctx_offset to
> > its true maximum offset, and can lead to out of bounds accesses. Second,
> > tail called program (or extension program replacing global subprog) where
> > their max_ctx_offset exceeds the program they are being called from can
> > also cause issues. For the latter, unmodified PTR_TO_CTX is the first
> > requirement for the fix, the second is ensuring max_ctx_offset >= the
> > program they are being called from, which has to be a separate change
> > not made in this commit.
> >
> > All in all, we can hint using arg_type when we expect ARG_PTR_TO_CTX and
> > make our relaxation around offsets conditional on it.
> >
> > Cc: Tejun Heo <tj@kernel.org>
> > Cc: Dan Schatzberg <dschatzberg@meta.com>
> > Reviewed-by: Emil Tsalapatis <emil@etsalapatis.com>
> > Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
> > ---
> >  kernel/bpf/verifier.c                         | 67 ++++++++++++-------
> >  .../bpf/progs/verifier_global_subprogs.c      |  1 -
> >  2 files changed, 43 insertions(+), 25 deletions(-)
> >
> > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> > index 01c18f4268de..14bf64e0c600 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,16 +7858,26 @@ 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;
> > -
> >               /*
> >                * 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 allowed to be variable, then the maximum
> > +              * possible offset is simulated (which is equal to var_off.value
> > +              * when var_off is constant).
> >                */
> > -             if (fixed_off_ok)
> > -                     off += reg->var_off.value;
> > +             if (var_off_ok) {
> > +                     err = check_mem_region_access(env, regno, off, size, U16_MAX, false);
> > +                     if (err)
> > +                             return err;
> > +                     off += reg->umax_value;
> > +             } else {
> > +                     err = __check_ptr_off_reg(env, reg, regno, fixed_off_ok);
> > +                     if (err < 0)
> > +                             return err;
> > +                     if (fixed_off_ok)
> > +                             off += reg->var_off.value;
> > +             }
> nit: this code looks a bit complex, I wonder if it makes sense to encode
> the context offset mode into an enum:
> enum bpf_ctx_allowed_off {
>      CTX_OFF_VAR,
>      CTX_OFF_FIXED,
>      CTX_OFF_NONE
> };
>
> we can factor out a helper that returns allowed offset mode:
> ```
> enum bpf_ctx_allowed_off get_context_allowed_offset(env)
> {
>         if (resolve_prog_type(env->prog) == BPF_PROG_TYPE_SYSCALL)
>                 return CTX_OFF_VAR;
>         else if (!env->ops->convert_ctx_access)
>                 return CTX_OFF_FIXED;
>         else
>                 return CTX_OFF_NONE;
> }
> ```
>
> The enum makes the three-way exclusive modes explicit, eliminates the
> implicit priority and more self-documenting.
>
> The enum can also be used below.

Take a look at Eduard's suggestion, and let me know your thoughts.

>
> > +
> >               err = check_ctx_access(env, insn_idx, off, size, t, &info);
> >               if (err)
> >                       verbose_linfo(env, insn_idx, "; ");
> > @@ -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) {
> Why did you remove this block here, it should correspond to fixed
> offset, and is not processed in the resolve_prog_type(env->prog) ==
> BPF_PROG_TYPE_SYSCALL) branch.

Because I don't know if it makes any sense to support passing ctx for
other program types except syscall into helpers.
I noted this in the commit message. I would be ok to change it back,
but I think passing ctx into helpers as memory is questionable for
anything except syscall progs.

> > -                     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) {
> If we introduce bpf_ctx_allowed_off enum, this check could be modified
> to if (get_context_allowed_offset() == CTX_OFF_VAR) here and also in
> other place as well, does it capture the logic better?
> I'm not 100% sure if these use cases are worth adding a separate enum,
> though, let me know what you think.
> > +                     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,17 @@ 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, but
> > +              * only when the argument is passed as memory, not ctx,
> > +              * otherwise we may get modified ctx in tail called programs and
> > +              * global subprogs (that may act as extension prog hooks).
> > +              */
> > +             if (arg_type != ARG_PTR_TO_CTX &&
> > +                 resolve_prog_type(env->prog) == BPF_PROG_TYPE_SYSCALL)
> This looks like another instance where we check for prog_type==syscall,
> but actually mean: is variable offset into ctx is allowed.
> > +                     return 0;
> > +             fallthrough;
> >       default:
> >               return __check_ptr_off_reg(env, reg, regno, false);
> >       }
> > @@ -10757,7 +10774,7 @@ static int btf_check_func_arg_match(struct bpf_verifier_env *env, int subprog,
> >                        * invalid memory access.
> >                        */
> >               } else if (arg->arg_type == ARG_PTR_TO_CTX) {
> > -                     ret = check_func_arg_reg_off(env, reg, regno, ARG_DONTCARE);
> > +                     ret = check_func_arg_reg_off(env, reg, regno, ARG_PTR_TO_CTX);
> >                       if (ret < 0)
> >                               return ret;
> >                       /* If function expects ctx type in BTF check that caller
> > @@ -13565,7 +13582,6 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_kfunc_call_
> >                               }
> >                       }
> >                       fallthrough;
> > -             case KF_ARG_PTR_TO_CTX:
> >               case KF_ARG_PTR_TO_DYNPTR:
> >               case KF_ARG_PTR_TO_ITER:
> >               case KF_ARG_PTR_TO_LIST_HEAD:
> > @@ -13583,6 +13599,9 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_kfunc_call_
> >               case KF_ARG_PTR_TO_IRQ_FLAG:
> >               case KF_ARG_PTR_TO_RES_SPIN_LOCK:
> >                       break;
> > +             case KF_ARG_PTR_TO_CTX:
> > +                     arg_type = ARG_PTR_TO_CTX;
> > +                     break;
> >               default:
> >                       verifier_bug(env, "unknown kfunc arg type %d", kf_arg_type);
> >                       return -EFAULT;
> > 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)
> >  {
> > --
> > 2.52.0

Re: [PATCH bpf-next v3 2/3] selftests/bpf: Add syscall ctx variable offset tests

[Kumar Kartikeya Dwivedi]

On Wed, 18 Mar 2026 at 15:45, Mykyta Yatsenko
<mykyta.yatsenko5@gmail.com> wrote:
>
> Kumar Kartikeya Dwivedi <memxor@gmail.com> writes:
>
> > Add various tests to exercise fixed and variable offsets on PTR_TO_CTX
> > for syscall programs, and cover disallowed cases for other program types
> > lacking convert_ctx_access callback. Load verifier_ctx with CAP_SYS_ADMIN
> > so that kfunc related logic can be tested. While at it, convert assembly
> > tests to C.
> >
> > Reviewed-by: Emil Tsalapatis <emil@etsalapatis.com>
> > Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
> > ---
> >  .../selftests/bpf/prog_tests/verifier.c       |  34 +-
> >  .../selftests/bpf/progs/verifier_ctx.c        | 356 +++++++++++++++---
> >  .../selftests/bpf/test_kmods/bpf_testmod.c    |   2 +-
> >  3 files changed, 325 insertions(+), 67 deletions(-)
> >
> > diff --git a/tools/testing/selftests/bpf/prog_tests/verifier.c b/tools/testing/selftests/bpf/prog_tests/verifier.c
> > index 8cdfd74c95d7..04d5f46264a3 100644
> > --- a/tools/testing/selftests/bpf/prog_tests/verifier.c
> > +++ b/tools/testing/selftests/bpf/prog_tests/verifier.c
> > @@ -126,29 +126,35 @@ struct test_val {
> >  __maybe_unused
> >  static void run_tests_aux(const char *skel_name,
> >                         skel_elf_bytes_fn elf_bytes_factory,
> > -                       pre_execution_cb pre_execution_cb)
> > +                       pre_execution_cb pre_execution_cb,
> > +                       bool drop_sys_admin)
> >  {
> >       struct test_loader tester = {};
> >       __u64 old_caps;
> >       int err;
> >
> > -     /* test_verifier tests are executed w/o CAP_SYS_ADMIN, do the same here */
> > -     err = cap_disable_effective(1ULL << CAP_SYS_ADMIN, &old_caps);
> > -     if (err) {
> > -             PRINT_FAIL("failed to drop CAP_SYS_ADMIN: %i, %s\n", err, strerror(-err));
> > -             return;
> > +     if (drop_sys_admin) {
> > +             /* test_verifier tests are executed w/o CAP_SYS_ADMIN, do the same here */
> > +             err = cap_disable_effective(1ULL << CAP_SYS_ADMIN, &old_caps);
> > +             if (err) {
> > +                     PRINT_FAIL("failed to drop CAP_SYS_ADMIN: %i, %s\n", err, strerror(-err));
> > +                     return;
> > +             }
> >       }
> >
> >       test_loader__set_pre_execution_cb(&tester, pre_execution_cb);
> >       test_loader__run_subtests(&tester, skel_name, elf_bytes_factory);
> >       test_loader_fini(&tester);
> >
> > -     err = cap_enable_effective(old_caps, NULL);
> > -     if (err)
> > -             PRINT_FAIL("failed to restore CAP_SYS_ADMIN: %i, %s\n", err, strerror(-err));
> > +     if (drop_sys_admin) {
> > +             err = cap_enable_effective(old_caps, NULL);
> > +             if (err)
> > +                     PRINT_FAIL("failed to restore CAP_SYS_ADMIN: %i, %s\n", err, strerror(-err));
> > +     }
> >  }
> >
> > -#define RUN(skel) run_tests_aux(#skel, skel##__elf_bytes, NULL)
> > +#define RUN(skel) run_tests_aux(#skel, skel##__elf_bytes, NULL, true)
> > +#define RUN_WITH_CAP_SYS_ADMIN(skel) run_tests_aux(#skel, skel##__elf_bytes, NULL, false)
> >
> >  void test_verifier_align(void)                { RUN(verifier_align); }
> >  void test_verifier_and(void)                  { RUN(verifier_and); }
> > @@ -173,7 +179,7 @@ void test_verifier_cgroup_skb(void)           { RUN(verifier_cgroup_skb); }
> >  void test_verifier_cgroup_storage(void)       { RUN(verifier_cgroup_storage); }
> >  void test_verifier_const(void)                { RUN(verifier_const); }
> >  void test_verifier_const_or(void)             { RUN(verifier_const_or); }
> > -void test_verifier_ctx(void)                  { RUN(verifier_ctx); }
> > +void test_verifier_ctx(void)                  { RUN_WITH_CAP_SYS_ADMIN(verifier_ctx); }
> >  void test_verifier_ctx_sk_msg(void)           { RUN(verifier_ctx_sk_msg); }
> >  void test_verifier_d_path(void)               { RUN(verifier_d_path); }
> >  void test_verifier_default_trusted_ptr(void)  { RUN_TESTS(verifier_default_trusted_ptr); }
> > @@ -293,7 +299,8 @@ void test_verifier_array_access(void)
> >  {
> >       run_tests_aux("verifier_array_access",
> >                     verifier_array_access__elf_bytes,
> > -                   init_array_access_maps);
> > +                   init_array_access_maps,
> > +                   true);
> >  }
> >  void test_verifier_async_cb_context(void)    { RUN(verifier_async_cb_context); }
> >
> > @@ -306,5 +313,6 @@ void test_verifier_value_ptr_arith(void)
> >  {
> >       run_tests_aux("verifier_value_ptr_arith",
> >                     verifier_value_ptr_arith__elf_bytes,
> > -                   init_value_ptr_arith_maps);
> > +                   init_value_ptr_arith_maps,
> > +                   true);
> >  }
> > diff --git a/tools/testing/selftests/bpf/progs/verifier_ctx.c b/tools/testing/selftests/bpf/progs/verifier_ctx.c
> > index 371780290c0d..c054c8cb7242 100644
> > --- a/tools/testing/selftests/bpf/progs/verifier_ctx.c
> > +++ b/tools/testing/selftests/bpf/progs/verifier_ctx.c
> > @@ -4,6 +4,10 @@
> >  #include "vmlinux.h"
> >  #include <bpf/bpf_helpers.h>
> >  #include "bpf_misc.h"
> > +#include "../test_kmods/bpf_testmod_kfunc.h"
> > +
> > +const char ctx_strncmp_target[] = "ctx";
> > +const char ctx_snprintf_fmt[] = "";
> >
> >  SEC("tc")
> >  __description("context stores via BPF_ATOMIC")
> > @@ -69,7 +73,6 @@ __naked void ctx_pointer_to_helper_1(void)
> >  SEC("socket")
> >  __description("pass modified ctx pointer to helper, 2")
> >  __failure __msg("negative offset ctx ptr R1 off=-612 disallowed")
> > -__failure_unpriv __msg_unpriv("negative offset ctx ptr R1 off=-612 disallowed")
> >  __naked void ctx_pointer_to_helper_2(void)
> >  {
> >       asm volatile ("                                 \
> > @@ -295,77 +298,324 @@ padding_access("sk_reuseport", sk_reuseport_md, hash, 4);
> >  SEC("syscall")
> >  __description("syscall: write to ctx with fixed offset")
> >  __success
> > -__naked void syscall_ctx_fixed_off_write(void)
> > +int syscall_ctx_fixed_off_write(void *ctx)
> >  {
> > -     asm volatile ("                                 \
> > -     r0 = 0;                                         \
> > -     *(u32*)(r1 + 0) = r0;                           \
> > -     r1 += 4;                                        \
> > -     *(u32*)(r1 + 0) = r0;                           \
> > -     exit;                                           \
> > -"    ::: __clobber_all);
> > +     char *p = ctx;
> > +
> > +     *(__u32 *)p = 0;
> > +     *(__u32 *)(p + 4) = 0;
> > +     return 0;
> > +}
> > +
> > +SEC("syscall")
> > +__description("syscall: read ctx with fixed offset")
> > +__success
> > +int syscall_ctx_fixed_off_read(void *ctx)
> > +{
> > +     char *p = ctx;
> > +     volatile __u32 val;
> > +
> > +     val = *(__u32 *)(p + 4);
> > +     (void)val;
> > +     return 0;
> > +}
> > +
> > +SEC("syscall")
> > +__description("syscall: read ctx with variable offset")
> > +__success
> > +int syscall_ctx_var_off_read(void *ctx)
> > +{
> > +     __u64 off = bpf_get_prandom_u32();
> > +     char *p = ctx;
> > +     volatile __u32 val;
> > +
> > +     off &= 0xfc;
> > +     p += off;
> > +     val = *(__u32 *)p;
> > +     (void)val;
> > +     return 0;
> > +}
> > +
> > +SEC("syscall")
> > +__description("syscall: write ctx with variable offset")
> > +__success
> > +int syscall_ctx_var_off_write(void *ctx)
> > +{
> > +     __u64 off = bpf_get_prandom_u32();
> > +     char *p = ctx;
> > +
> > +     off &= 0xfc;
> > +     p += off;
> > +     *(__u32 *)p = 0;
> > +     return 0;
> > +}
> > +
> > +SEC("syscall")
> > +__description("syscall: reject negative variable offset ctx access")
> > +__failure __msg("min value is negative")
> > +int syscall_ctx_neg_var_off(void *ctx)
> > +{
> > +     __u64 off = bpf_get_prandom_u32();
> > +     char *p = ctx;
> > +
> > +     off &= 4;
> > +     p -= off;
> > +     return *(__u32 *)p;
> > +}
> > +
> > +SEC("syscall")
> > +__description("syscall: reject unbounded variable offset ctx access")
> > +__failure __msg("unbounded memory access")
> > +int syscall_ctx_unbounded_var_off(void *ctx)
> > +{
> > +     __u64 off = (__u32)bpf_get_prandom_u32();
> > +     char *p = ctx;
> > +
> > +     off <<= 2;
> > +     p += off;
> > +     return *(__u32 *)p;
> > +}
> > +
> > +SEC("syscall")
> > +__description("syscall: helper read ctx with fixed offset")
> > +__success
> > +int syscall_ctx_helper_fixed_off_read(void *ctx)
> > +{
> > +     char *p = ctx;
> > +
> > +     p += 4;
> > +     return bpf_strncmp(p, 4, ctx_strncmp_target);
> > +}
> > +
> > +SEC("syscall")
> > +__description("syscall: helper write ctx with fixed offset")
> > +__success
> > +int syscall_ctx_helper_fixed_off_write(void *ctx)
> > +{
> > +     char *p = ctx;
> > +
> > +     p += 4;
> > +     return bpf_probe_read_kernel(p, 4, 0);
> > +}
> > +
> > +SEC("syscall")
> > +__description("syscall: helper read ctx with variable offset")
> > +__success
> > +int syscall_ctx_helper_var_off_read(void *ctx)
> > +{
> > +     __u64 off = bpf_get_prandom_u32();
> > +     char *p = ctx;
> > +
> > +     off &= 0xfc;
> > +     p += off;
> > +     return bpf_strncmp(p, 4, ctx_strncmp_target);
> > +}
> > +
> > +SEC("syscall")
> > +__description("syscall: helper write ctx with variable offset")
> > +__success
> > +int syscall_ctx_helper_var_off_write(void *ctx)
> > +{
> > +     __u64 off = bpf_get_prandom_u32();
> > +     char *p = ctx;
> > +
> > +     off &= 0xfc;
> > +     p += off;
> > +     return bpf_probe_read_kernel(p, 4, 0);
> > +}
> > +
> > +SEC("syscall")
> > +__description("syscall: helper read zero-sized ctx access")
> > +__success
> > +int syscall_ctx_helper_zero_sized_read(void *ctx)
> > +{
> > +     return bpf_snprintf(0, 0, ctx_snprintf_fmt, ctx, 0);
> > +}
> > +
> > +SEC("syscall")
> > +__description("syscall: helper write zero-sized ctx access")
> > +__success
> > +int syscall_ctx_helper_zero_sized_write(void *ctx)
> > +{
> > +     return bpf_probe_read_kernel(ctx, 0, 0);
> > +}
> > +
> > +SEC("syscall")
> > +__description("syscall: kfunc access ctx with fixed offset")
> > +__success
> > +int syscall_ctx_kfunc_fixed_off(void *ctx)
> > +{
> > +     char *p = ctx;
> > +
> > +     p += 4;
> > +     bpf_kfunc_call_test_mem_len_pass1(p, 4);
> > +     return 0;
> > +}
> > +
> > +SEC("syscall")
> > +__description("syscall: kfunc access ctx with variable offset")
> > +__success
> > +int syscall_ctx_kfunc_var_off(void *ctx)
> > +{
> > +     __u64 off = bpf_get_prandom_u32();
> > +     char *p = ctx;
> > +
> > +     off &= 0xfc;
> > +     p += off;
> > +     bpf_kfunc_call_test_mem_len_pass1(p, 4);
> > +     return 0;
> > +}
> > +
> > +SEC("syscall")
> > +__description("syscall: kfunc access zero-sized ctx")
> > +__success
> > +int syscall_ctx_kfunc_zero_sized(void *ctx)
> > +{
> > +     bpf_kfunc_call_test_mem_len_pass1(ctx, 0);
> > +     return 0;
> >  }
> Tests are comprehensive. Do we need to test rejection of unaligned read
> too?
> Acked-by: Mykyta Yatsenko <yatsenko@meta.com>

Thanks, I am actually now considering dropping the aligned access
requirement, I will add a test for it.

> [...]

Re: [PATCH bpf-next v3 1/3] bpf: Support variable offsets for syscall PTR_TO_CTX

[Eduard Zingerman]

On Wed, 2026-03-18 at 22:57 +0100, Kumar Kartikeya Dwivedi wrote:

[...]

> > Let's simplify this as:
> > 
> >                 if (var_off_ok)
> >                         err = check_mem_region_access(env, regno, off, size, U16_MAX, false);
> >                 else
> >                         err = __check_ptr_off_reg(env, reg, regno, fixed_off_ok);
> >                 if (err < 0)
> >                         return err;
> >                 off += reg->umax_value;
> 
> Ack.
> 
> > 
> > Tbh, the above looks a bit ad-hoc, one might argue that a better way in
> > terms of abstraction would be to adjust bpf_verifier_ops->is_valid_access()
> > to accept or reject varying offset access.
> 
> I think for all current cases, the callback is supplied to make sure
> the fixed offset is well formed.
> There isn't really "valid or invalid" access otherwise, esp. for
> syscall programs.
> We can probably also drop this from syscall_prog_is_valid_access.
> > if (off % size != 0)
> Unaligned accesses should be fine.

You mean exception handler would catch it?

> If we moved it inside we'd have to repeat this ptr offset thing for
> most of them except syscall.
> Second, it does not have access to either env or reg to do the
> check_mem_region_access.

Yes, this will include some mechanical changes.
Ok, let's stick with special case for now.

> > 
> > >               err = check_ctx_access(env, insn_idx, off, size, t, &info);
> > >               if (err)
> > >                       verbose_linfo(env, insn_idx, "; ");
> > > @@ -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) {
> > 
> > Nit: this is a second place where this check is hard-coded.
> 
> Any better ideas? This program type is sort of special wrt semantics of ctx.
> Unlike all others, ctx here is user supplied, and rw, and carries no
> kernel data.

We usually hide such things behind a utility function,
e.g. is_varying_context_offset_allowed(env).

> 
> > 
> > > +                     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;
> > 
> > Why is this change necessary at all? Wouldn't old code handle all
> > relevant cases, now that check_mem_access() is updated?
> 
> I think we cannot pass the access_size through, check_mem_access is
> meant to simulate an instruction access.
> Earlier it was doing access on the final byte since pointer
> modification was not permitted, so real register offset was always
> zero.
> All we needed to check was whether access_size - 1 fell within bounds,
> more importantly just record the max_ctx_offset since there's no
> bounds except U16_MAX.
> Now, the register is modified, and we can access lengths wider than
> those we get from bpf_size_to_bytes.

Oh, I see. Would it be possible to extract a utility function such
that it is used both from here and from check_mem_access?

> > 
> > >               }
> > > -
> > >               fallthrough;
> > >       default: /* scalar_value or invalid ptr */
> > >               /* Allow zero-byte read from NULL, regardless of pointer type */
> > 
> > [...]
> > 
> > > 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")
> > 
> > Nit: should this test case be forked as tracepoint and non-tracepoint versions?
> 
> In terms of coverage? I have those in the next patch already.

Ack.

Re: [PATCH bpf-next v3 1/3] bpf: Support variable offsets for syscall PTR_TO_CTX

[Kumar Kartikeya Dwivedi]

On Wed, 18 Mar 2026 at 23:37, Eduard Zingerman <eddyz87@gmail.com> wrote:
>
> On Wed, 2026-03-18 at 22:57 +0100, Kumar Kartikeya Dwivedi wrote:
>
> [...]
>
> > > Let's simplify this as:
> > >
> > >                 if (var_off_ok)
> > >                         err = check_mem_region_access(env, regno, off, size, U16_MAX, false);
> > >                 else
> > >                         err = __check_ptr_off_reg(env, reg, regno, fixed_off_ok);
> > >                 if (err < 0)
> > >                         return err;
> > >                 off += reg->umax_value;
> >
> > Ack.
> >
> > >
> > > Tbh, the above looks a bit ad-hoc, one might argue that a better way in
> > > terms of abstraction would be to adjust bpf_verifier_ops->is_valid_access()
> > > to accept or reject varying offset access.
> >
> > I think for all current cases, the callback is supplied to make sure
> > the fixed offset is well formed.
> > There isn't really "valid or invalid" access otherwise, esp. for
> > syscall programs.
> > We can probably also drop this from syscall_prog_is_valid_access.
> > > if (off % size != 0)
> > Unaligned accesses should be fine.
>
> You mean exception handler would catch it?

lol, that would be bad. It probably wouldn't if it happened.
But I don't think that unaligned accesses will cause faults.
They were there to ensure field accesses were aligned correctly for
other ctx types.
This check is repeated elsewhere, and probably aped in this callback as well.

>
> > If we moved it inside we'd have to repeat this ptr offset thing for
> > most of them except syscall.
> > Second, it does not have access to either env or reg to do the
> > check_mem_region_access.
>
> Yes, this will include some mechanical changes.
> Ok, let's stick with special case for now.
>
> > >
> > > >               err = check_ctx_access(env, insn_idx, off, size, t, &info);
> > > >               if (err)
> > > >                       verbose_linfo(env, insn_idx, "; ");
> > > > @@ -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) {
> > >
> > > Nit: this is a second place where this check is hard-coded.
> >
> > Any better ideas? This program type is sort of special wrt semantics of ctx.
> > Unlike all others, ctx here is user supplied, and rw, and carries no
> > kernel data.
>
> We usually hide such things behind a utility function,
> e.g. is_varying_context_offset_allowed(env).
>

Ok, I will wrap it.

> >
> > >
> > > > +                     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;
> > >
> > > Why is this change necessary at all? Wouldn't old code handle all
> > > relevant cases, now that check_mem_access() is updated?
> >
> > I think we cannot pass the access_size through, check_mem_access is
> > meant to simulate an instruction access.
> > Earlier it was doing access on the final byte since pointer
> > modification was not permitted, so real register offset was always
> > zero.
> > All we needed to check was whether access_size - 1 fell within bounds,
> > more importantly just record the max_ctx_offset since there's no
> > bounds except U16_MAX.
> > Now, the register is modified, and we can access lengths wider than
> > those we get from bpf_size_to_bytes.
>
> Oh, I see. Would it be possible to extract a utility function such
> that it is used both from here and from check_mem_access?
>
> > >
> > > >               }
> > > > -
> > > >               fallthrough;
> > > >       default: /* scalar_value or invalid ptr */
> > > >               /* Allow zero-byte read from NULL, regardless of pointer type */
> > >
> > > [...]
> > >
> > > > 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")
> > >
> > > Nit: should this test case be forked as tracepoint and non-tracepoint versions?
> >
> > In terms of coverage? I have those in the next patch already.
>
> Ack.

Re: [PATCH bpf-next v3 1/3] bpf: Support variable offsets for syscall PTR_TO_CTX

[Kumar Kartikeya Dwivedi]

On Wed, 18 Mar 2026 at 23:37, Eduard Zingerman <eddyz87@gmail.com> wrote:
>
> [...]
> > I think we cannot pass the access_size through, check_mem_access is
> > meant to simulate an instruction access.
> > Earlier it was doing access on the final byte since pointer
> > modification was not permitted, so real register offset was always
> > zero.
> > All we needed to check was whether access_size - 1 fell within bounds,
> > more importantly just record the max_ctx_offset since there's no
> > bounds except U16_MAX.
> > Now, the register is modified, and we can access lengths wider than
> > those we get from bpf_size_to_bytes.
>
> Oh, I see. Would it be possible to extract a utility function such
> that it is used both from here and from check_mem_access?
>

Pressed enter too quickly.
I think we can do it, the only difference probably would be the
access_size passed down both.

But looking closer at it now, should syscall_prog_is_valid_access be
doing off + size >= U16_MAX?
If off is U16_MAX - 1, and size is >BPF_B, we will likely set
max_ctx_offset beyond U16_MAX, which is the maximum permitted size.
This is through the instruction path now, through the helper path I
encoded U16_MAX as the max size.

We probably can end up doing OOB read, since bpf_prog_test_run_syscall
only checks ctx_size_in > U16_MAX and errors out, but relies on
max_ctx_offset to be <= that.

> [...]