[PATCH bpf-next 00/12] bpf: Support by-value struct and __int128 arguments

[PATCH bpf-next 00/12] bpf: Support by-value struct and __int128 arguments

[Yonghong Song]

A global function or a kfunc taking a struct or union by value is rejected
today:

  Arg#1 type STRUCT in tar() is not supported yet.
  Unrecognized R2 type STRUCT

and an __int128 argument is accepted but mis-counted: the compiler passes
it in two registers while the verifier gives it one, so every argument
after it is checked against the wrong register and the program is rejected
for a register its source never names.

The compiler passes such a value in one argument register per eightbyte,
so a value of at most 16 bytes arrives in one or two consecutive
registers. This series teaches the verifier to describe that: sub->args[]
gains one entry per argument slot rather than one per BTF parameter, and
the kfunc argument walk counts slots the same way. A value may only be
composed of scalars, as a pointer would reach the callee as an opaque
scalar, losing the provenance and reference tracking that make it safe to
use. A global function still has no stack arguments, so all of its slots
have to fit in the argument registers.

For a kfunc the two calling conventions have to agree, and they do not
always. Where the BPF convention splits a value between the last argument
register and the stack, SysV moves the whole of it to the stack and gives
the register to the argument that follows, while AAPCS64 rounds the
register number up to an even one for a 16-byte aligned value and gives no
register to anything once one has gone to the stack. The JITs move the
eightbytes the two conventions place differently, and
bpf_jit_supports_kfunc_arg_slot() asks the JIT whether it can, defaulting
to no, so such an argument is refused on an architecture that has not
implemented the placement:

  Function f arg#5 type STRUCT cannot be passed at argument slot 5 on this
  architecture

Patch 1 records the __int128 failure as it stands today, which patch 4
turns into a success. Patches 2 to 4 index the arguments of a global
function by slot and accept a by-value struct and an __int128. Patch 5
does the same for a kfunc call. Patches 6 to 9 place the arguments per the
kernel calling convention in the x86-64 and arm64 JITs. Patches 10 to 12
are the tests.

Tested on x86-64 and arm64 with test_progs. The by-value struct tests are
built with clang, as GCC passes an aggregate by invisible reference, and
some of them need __BPF_FEATURE_STACK_ARGUMENT.

Yonghong Song (12):
  selftests/bpf: Add a test for an __int128 by-value argument
  bpf: Index global function arguments by argument slot
  bpf: Support by-value struct arguments up to 16 bytes
  bpf: Support __int128 as a by-value function argument
  bpf: Support by-value struct and __int128 kfunc arguments
  bpf: Add a JIT helper for the outgoing stack of kfunc calls
  bpf, x86: Place kfunc arguments per the SysV calling convention
  bpf: Record a 16-byte argument alignment in the function model
  bpf, arm64: Place kfunc arguments per AAPCS64
  selftests/bpf: Add C tests for by-value arguments up to 16 bytes
  selftests/bpf: Add inline-asm tests for by-value arguments
  selftests/bpf: Add tests for by-value kfunc arguments

 arch/arm64/net/bpf_jit_comp.c                 | 112 +++++++++-
 arch/x86/net/bpf_jit_comp.c                   | 185 +++++++++++++++-
 include/linux/bpf.h                           |   3 +
 include/linux/bpf_verifier.h                  |   1 +
 include/linux/filter.h                        |   3 +
 kernel/bpf/btf.c                              |  98 +++++++--
 kernel/bpf/core.c                             |  42 ++++
 kernel/bpf/verifier.c                         | 200 ++++++++++++++++--
 .../selftests/bpf/prog_tests/aggregate_arg.c  |  11 +
 .../selftests/bpf/prog_tests/verifier.c       |   2 +
 .../selftests/bpf/progs/aggregate_arg_func.c  | 155 ++++++++++++++
 .../selftests/bpf/progs/aggregate_arg_kfunc.c | 106 ++++++++++
 .../testing/selftests/bpf/progs/arena_kfunc.c |  16 ++
 .../selftests/bpf/progs/stack_arg_fail.c      |  10 -
 .../selftests/bpf/progs/verifier_int128_arg.c | 195 +++++++++++++++++
 .../selftests/bpf/test_kmods/bpf_testmod.c    |  40 ++++
 .../bpf/test_kmods/bpf_testmod_kfunc.h        |  22 ++
 17 files changed, 1151 insertions(+), 50 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/aggregate_arg.c
 create mode 100644 tools/testing/selftests/bpf/progs/aggregate_arg_func.c
 create mode 100644 tools/testing/selftests/bpf/progs/aggregate_arg_kfunc.c
 create mode 100644 tools/testing/selftests/bpf/progs/verifier_int128_arg.c

-- 
2.53.0-Meta

[PATCH bpf-next 01/12] selftests/bpf: Add a test for an __int128 by-value argument

[Yonghong Song]

A 128-bit integer is passed in two consecutive argument registers, but
the verifier counts one argument register per parameter whatever its
size. For

  __u64 take_i128_global(int a, u128 v, int c)

the compiler passes a in R1, v in R2:R3 and c in R4, while the verifier
marks only R1 through R3 at the entry of the global function. The callee
then reads its own third parameter out of a register the verifier
considers uninitialized, and the program is rejected for a register the
source never names:

  Validating take_i128_global() func#1...
  20: R1=scalar() R2=scalar() R3=scalar() R10=fp0
  ; __noinline __u64 take_i128_global(int a, u128 v, int c) @ verifier_int128_arg.c:12
  20: (bf) r0 = r2                      ; R0=scalar(id=4) R2=scalar(id=4)
  ; return (__u64)a + (__u64)(v >> 64) + (__u64)v + c; @ verifier_int128_arg.c:14
  21: (bc) w1 = w1                      ; R1=scalar(smin=0,smax=umax=0xffffffff,var_off=(0x0; 0xffffffff))
  22: (67) r1 <<= 32                    ; R1=scalar(smax=0x7fffffff00000000,smin32=0,smax32=umax32=0,var_off=(0x0; 0xffffffff00000000))
  23: (c7) r1 s>>= 32                   ; R1=scalar(smin=0xffffffff80000000,smax=0x7fffffff)
  24: (0f) r0 += r1                     ; R0=scalar() R1=scalar(smin=0xffffffff80000000,smax=0x7fffffff)
  25: (0f) r0 += r3                     ; R0=scalar() R3=scalar()
  26: (bc) w1 = w4
  R4 !read_ok

The log is from clang 23. LLVM 21/22 place the argument in the same
registers.

Add the test with the failure it produces now. A later patch will fix
this issue so this test should succeed.

Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
---
 .../selftests/bpf/prog_tests/verifier.c       |  2 ++
 .../selftests/bpf/progs/verifier_int128_arg.c | 36 +++++++++++++++++++
 2 files changed, 38 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/progs/verifier_int128_arg.c

diff --git a/tools/testing/selftests/bpf/prog_tests/verifier.c b/tools/testing/selftests/bpf/prog_tests/verifier.c
index f7f94ccebce2..0d68b92d6692 100644
--- a/tools/testing/selftests/bpf/prog_tests/verifier.c
+++ b/tools/testing/selftests/bpf/prog_tests/verifier.c
@@ -50,6 +50,7 @@
 #include "verifier_helper_packet_access.skel.h"
 #include "verifier_helper_restricted.skel.h"
 #include "verifier_helper_value_access.skel.h"
+#include "verifier_int128_arg.skel.h"
 #include "verifier_int_ptr.skel.h"
 #include "verifier_iterating_callbacks.skel.h"
 #include "verifier_jeq_infer_not_null.skel.h"
@@ -214,6 +215,7 @@ void test_verifier_helper_access_var_len(void) { RUN(verifier_helper_access_var_
 void test_verifier_helper_packet_access(void) { RUN(verifier_helper_packet_access); }
 void test_verifier_helper_restricted(void)    { RUN(verifier_helper_restricted); }
 void test_verifier_helper_value_access(void)  { RUN(verifier_helper_value_access); }
+void test_verifier_int128_arg(void)           { RUN_TESTS(verifier_int128_arg); }
 void test_verifier_int_ptr(void)              { RUN(verifier_int_ptr); }
 void test_verifier_iterating_callbacks(void)  { RUN(verifier_iterating_callbacks); }
 void test_verifier_jeq_infer_not_null(void)   { RUN(verifier_jeq_infer_not_null); }
diff --git a/tools/testing/selftests/bpf/progs/verifier_int128_arg.c b/tools/testing/selftests/bpf/progs/verifier_int128_arg.c
new file mode 100644
index 000000000000..419851f2d8d0
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/verifier_int128_arg.c
@@ -0,0 +1,36 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2026 Meta Platforms, Inc. and affiliates. */
+#include <vmlinux.h>
+#include <bpf/bpf_helpers.h>
+#include "bpf_misc.h"
+
+#define MIX_A	0xdeadbeefcafef00dULL
+#define MIX_B	0x0123456789abcdefULL
+
+typedef unsigned __int128 u128;
+
+__noinline __u64 take_i128_global(int a, u128 v, int c)
+{
+	return (__u64)a + (__u64)(v >> 64) + (__u64)v + c;
+}
+
+SEC("tc")
+/*
+ * The verifier counts one argument register for the __int128 and marks only
+ * R1 through R3 at the entry of take_i128_global(), while the compiler passed
+ * a in R1, v in R2:R3 and c in R4.
+ */
+__failure __msg("R4 !read_ok")
+int aggregate_arg_int128_c_test(struct __sk_buff *skb)
+{
+	__u64 a = skb->len ^ MIX_A;
+	__u64 b = skb->len ^ MIX_B;
+	u128 v = ((u128)a << 64) | b;
+
+	if (take_i128_global(1, v, 2) != a + b + 3)
+		return 1;
+
+	return 0;
+}
+
+char _license[] SEC("license") = "GPL";
-- 
2.53.0-Meta

[PATCH bpf-next 02/12] bpf: Index global function arguments by argument slot

[Yonghong Song]

btf_prepare_func_args() indexes sub->args[] by BTF parameter, so a
parameter can only ever stand for a single argument register. Give the
loop a second index: 'i' keeps walking the BTF parameters while
'slots_used' walks the argument slots, and sub->arg_cnt becomes the
number of slots, so that a later patch can give a parameter two of them.

No functional change: every parameter still takes exactly one slot.

Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
---
 kernel/bpf/btf.c | 40 ++++++++++++++++++++++++----------------
 1 file changed, 24 insertions(+), 16 deletions(-)

diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index 9c2cab08bb79..22828b489b77 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -8015,7 +8015,7 @@ int btf_prepare_func_args(struct bpf_verifier_env *env, int subprog)
 	const struct btf_param *args;
 	const struct btf_type *t, *ref_t, *fn_t;
 	int err;
-	u32 i, nargs, btf_id;
+	u32 i, slots_used, nargs, btf_id;
 	const char *tname;
 
 	if (sub->args_cached)
@@ -8100,8 +8100,9 @@ int btf_prepare_func_args(struct bpf_verifier_env *env, int subprog)
 	/* Convert BTF function arguments into verifier types.
 	 * Only PTR_TO_CTX and SCALAR are supported atm.
 	 */
-	for (i = 0; i < nargs; i++) {
+	for (i = 0, slots_used = 0; i < nargs; i++) {
 		u32 tags = 0;
+
 		err = btf_scan_decl_tags(env, btf, fn_t, i, is_global, &tags);
 		if (err)
 			return err;
@@ -8123,7 +8124,7 @@ int btf_prepare_func_args(struct bpf_verifier_env *env, int subprog)
 			    btf_validate_prog_ctx_type(log, btf, t, i, prog_type,
 						       prog->expected_attach_type))
 				return -EINVAL;
-			sub->args[i].arg_type = ARG_PTR_TO_CTX;
+			sub->args[slots_used++].arg_type = ARG_PTR_TO_CTX;
 			continue;
 		}
 		if (btf_is_dynptr_ptr(btf, t)) {
@@ -8131,7 +8132,7 @@ int btf_prepare_func_args(struct bpf_verifier_env *env, int subprog)
 				bpf_log(log, "arg#%d has invalid combination of tags\n", i);
 				return -EINVAL;
 			}
-			sub->args[i].arg_type = ARG_PTR_TO_DYNPTR;
+			sub->args[slots_used++].arg_type = ARG_PTR_TO_DYNPTR;
 			continue;
 		}
 		if (tags & ARG_TAG_TRUSTED) {
@@ -8146,10 +8147,11 @@ int btf_prepare_func_args(struct bpf_verifier_env *env, int subprog)
 			if (kern_type_id < 0)
 				return kern_type_id;
 
-			sub->args[i].arg_type = ARG_PTR_TO_BTF_ID | PTR_TRUSTED;
+			sub->args[slots_used].arg_type = ARG_PTR_TO_BTF_ID | PTR_TRUSTED;
 			if (tags & ARG_TAG_NULLABLE)
-				sub->args[i].arg_type |= PTR_MAYBE_NULL;
-			sub->args[i].btf_id = kern_type_id;
+				sub->args[slots_used].arg_type |= PTR_MAYBE_NULL;
+			sub->args[slots_used].btf_id = kern_type_id;
+			slots_used++;
 			continue;
 		}
 		if (tags & ARG_TAG_UNTRUSTED) {
@@ -8163,8 +8165,10 @@ int btf_prepare_func_args(struct bpf_verifier_env *env, int subprog)
 
 			ref_t = btf_type_skip_modifiers(btf, t->type, NULL);
 			if (btf_type_is_void(ref_t) || btf_type_is_primitive(ref_t)) {
-				sub->args[i].arg_type = ARG_PTR_TO_MEM | MEM_RDONLY | PTR_UNTRUSTED;
-				sub->args[i].mem_size = 0;
+				sub->args[slots_used].arg_type = ARG_PTR_TO_MEM | MEM_RDONLY |
+								 PTR_UNTRUSTED;
+				sub->args[slots_used].mem_size = 0;
+				slots_used++;
 				continue;
 			}
 
@@ -8180,8 +8184,9 @@ int btf_prepare_func_args(struct bpf_verifier_env *env, int subprog)
 					i, btf_type_str(ref_t), tname);
 				return -EINVAL;
 			}
-			sub->args[i].arg_type = ARG_PTR_TO_BTF_ID | PTR_UNTRUSTED;
-			sub->args[i].btf_id = kern_type_id;
+			sub->args[slots_used].arg_type = ARG_PTR_TO_BTF_ID | PTR_UNTRUSTED;
+			sub->args[slots_used].btf_id = kern_type_id;
+			slots_used++;
 			continue;
 		}
 		if (tags & ARG_TAG_ARENA) {
@@ -8189,7 +8194,7 @@ int btf_prepare_func_args(struct bpf_verifier_env *env, int subprog)
 				bpf_log(log, "arg#%d arena cannot be combined with any other tags\n", i);
 				return -EINVAL;
 			}
-			sub->args[i].arg_type = ARG_PTR_TO_ARENA;
+			sub->args[slots_used++].arg_type = ARG_PTR_TO_ARENA;
 			continue;
 		}
 		if (is_global) { /* generic user data pointer */
@@ -8209,10 +8214,11 @@ int btf_prepare_func_args(struct bpf_verifier_env *env, int subprog)
 				return -EINVAL;
 			}
 
-			sub->args[i].arg_type = ARG_PTR_TO_MEM | PTR_MAYBE_NULL;
+			sub->args[slots_used].arg_type = ARG_PTR_TO_MEM | PTR_MAYBE_NULL;
 			if (tags & ARG_TAG_NONNULL)
-				sub->args[i].arg_type &= ~PTR_MAYBE_NULL;
-			sub->args[i].mem_size = mem_size;
+				sub->args[slots_used].arg_type &= ~PTR_MAYBE_NULL;
+			sub->args[slots_used].mem_size = mem_size;
+			slots_used++;
 			continue;
 		}
 
@@ -8222,7 +8228,7 @@ int btf_prepare_func_args(struct bpf_verifier_env *env, int subprog)
 			return -EINVAL;
 		}
 		if (btf_type_is_int(t) || btf_is_any_enum(t)) {
-			sub->args[i].arg_type = ARG_ANYTHING;
+			sub->args[slots_used++].arg_type = ARG_ANYTHING;
 			continue;
 		}
 		if (!is_global)
@@ -8232,6 +8238,8 @@ int btf_prepare_func_args(struct bpf_verifier_env *env, int subprog)
 		return -EINVAL;
 	}
 
+	sub->arg_cnt = slots_used;
+
 	sub->args_cached = true;
 
 	return 0;
-- 
2.53.0-Meta

[PATCH bpf-next 03/12] bpf: Support by-value struct arguments up to 16 bytes

[Yonghong Song]

A global function taking a struct or union by value is rejected today:

  Arg#1 type STRUCT in tar() is not supported yet.

A struct of at most 16 bytes arrives in one or two consecutive
registers. Currently, the struct is composed of scalars.

The slots of a value are independent of each other, so the compiler may
split one across the last argument register and the stack, as in

  static void f(int a, int b, int c, int d, struct pair p);

or place it wholly past the registers, and the verifier describes either
the same way. What has to follow the slots is stack_arg_cnt, recomputed
from the slots consumed, together with the "no stack args in global
functions" and JIT support checks.

Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
---
 kernel/bpf/btf.c      | 48 +++++++++++++++++++++++++++++++++++++++++++
 kernel/bpf/verifier.c |  7 +++++++
 2 files changed, 55 insertions(+)

diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index 22828b489b77..dbd64cee1b27 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -8103,6 +8103,14 @@ int btf_prepare_func_args(struct bpf_verifier_env *env, int subprog)
 	for (i = 0, slots_used = 0; i < nargs; i++) {
 		u32 tags = 0;
 
+		if (slots_used >= MAX_BPF_FUNC_ARGS) {
+			if (!is_global)
+				return -EINVAL;
+			bpf_log(log, "Arguments of %s() need more than %d argument slots\n",
+				tname, MAX_BPF_FUNC_ARGS);
+			return -EINVAL;
+		}
+
 		err = btf_scan_decl_tags(env, btf, fn_t, i, is_global, &tags);
 		if (err)
 			return err;
@@ -8231,6 +8239,31 @@ int btf_prepare_func_args(struct bpf_verifier_env *env, int subprog)
 			sub->args[slots_used++].arg_type = ARG_ANYTHING;
 			continue;
 		}
+		if (btf_type_is_struct(t)) {
+			u32 nslots;
+
+			if (!t->size || t->size > 2 * BPF_REG_SIZE) {
+				if (!is_global)
+					return -EINVAL;
+				bpf_log(log,
+					"Arg#%d type %s in %s() has size %u, only 1 to %d bytes "
+					"can be passed by value\n",
+					i, btf_type_str(t), tname, t->size, 2 * BPF_REG_SIZE);
+				return -EINVAL;
+			}
+			if (!btf_struct_is_composed_of(env, btf, t, BTF_MEMBER_SCALAR)) {
+				if (!is_global)
+					return -EINVAL;
+				bpf_log(log, "Arg#%d type %s in %s() is not composed of scalars\n",
+					i, btf_type_str(t), tname);
+				return -EINVAL;
+			}
+
+			nslots = (t->size + BPF_REG_SIZE - 1) / BPF_REG_SIZE;
+			while (nslots--)
+				sub->args[slots_used++].arg_type = ARG_ANYTHING;
+			continue;
+		}
 		if (!is_global)
 			return -EINVAL;
 		bpf_log(log, "Arg#%d type %s in %s() is not supported yet.\n",
@@ -8238,6 +8271,21 @@ int btf_prepare_func_args(struct bpf_verifier_env *env, int subprog)
 		return -EINVAL;
 	}
 
+	if (slots_used > MAX_BPF_FUNC_REG_ARGS) {
+		if (is_global) {
+			bpf_log(log,
+				"global function %s() needs %d > %d argument slots, "
+				"stack args not supported\n",
+				tname, slots_used, MAX_BPF_FUNC_REG_ARGS);
+			return -EINVAL;
+		}
+		if (!bpf_jit_supports_stack_args()) {
+			bpf_log(log, "JIT does not support function %s() with %d argument slots\n",
+				tname, slots_used);
+			return -EFAULT;
+		}
+		sub->stack_arg_cnt = slots_used - MAX_BPF_FUNC_REG_ARGS;
+	}
 	sub->arg_cnt = slots_used;
 
 	sub->args_cached = true;
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 8f585ceb2cd5..17e576c7b757 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -9724,6 +9724,13 @@ static int btf_check_func_arg_match(struct bpf_verifier_env *env, int subprog,
 	func = btf_type_by_id(btf, env->prog->aux->func_info[subprog].type_id);
 	func_proto = btf_type_by_id(btf, func->type);
 	args = btf_params(func_proto);
+	/*
+	 * An argument slot index can run past the end of the parameter array,
+	 * so only pass the parameters, for the names of the stack arguments,
+	 * if the two match.
+	 */
+	if (sub->arg_cnt != btf_type_vlen(func_proto))
+		args = NULL;
 	ret = check_outgoing_stack_args(env, caller, sub->arg_cnt,
 					bpf_subprog_name(env, subprog), btf, args);
 	if (ret)
-- 
2.53.0-Meta

[PATCH bpf-next 04/12] bpf: Support __int128 as a by-value function argument

[Yonghong Song]

A 128-bit integer follows the same calling convention as a 16-byte
by-value struct: LLVM emits it as a 16-byte BTF_KIND_INT and passes it in
two consecutive argument registers. So a __int128 gets its two slots.

The test added at the start of the series, which recorded the wrong
register map as an R4 !read_ok rejection, now passes and is flipped to
__success.

Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
---
 kernel/bpf/btf.c                                        | 9 +++------
 tools/testing/selftests/bpf/progs/verifier_int128_arg.c | 7 +------
 2 files changed, 4 insertions(+), 12 deletions(-)

diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index dbd64cee1b27..6c391449e298 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -8235,11 +8235,7 @@ int btf_prepare_func_args(struct bpf_verifier_env *env, int subprog)
 			bpf_log(log, "arg#%d has pointer tag, but is not a pointer type\n", i);
 			return -EINVAL;
 		}
-		if (btf_type_is_int(t) || btf_is_any_enum(t)) {
-			sub->args[slots_used++].arg_type = ARG_ANYTHING;
-			continue;
-		}
-		if (btf_type_is_struct(t)) {
+		if (btf_type_is_int(t) || btf_is_any_enum(t) || btf_type_is_struct(t)) {
 			u32 nslots;
 
 			if (!t->size || t->size > 2 * BPF_REG_SIZE) {
@@ -8251,7 +8247,8 @@ int btf_prepare_func_args(struct bpf_verifier_env *env, int subprog)
 					i, btf_type_str(t), tname, t->size, 2 * BPF_REG_SIZE);
 				return -EINVAL;
 			}
-			if (!btf_struct_is_composed_of(env, btf, t, BTF_MEMBER_SCALAR)) {
+			if (btf_type_is_struct(t) &&
+			    !btf_struct_is_composed_of(env, btf, t, BTF_MEMBER_SCALAR)) {
 				if (!is_global)
 					return -EINVAL;
 				bpf_log(log, "Arg#%d type %s in %s() is not composed of scalars\n",
diff --git a/tools/testing/selftests/bpf/progs/verifier_int128_arg.c b/tools/testing/selftests/bpf/progs/verifier_int128_arg.c
index 419851f2d8d0..044fc4a80055 100644
--- a/tools/testing/selftests/bpf/progs/verifier_int128_arg.c
+++ b/tools/testing/selftests/bpf/progs/verifier_int128_arg.c
@@ -15,12 +15,7 @@ __noinline __u64 take_i128_global(int a, u128 v, int c)
 }
 
 SEC("tc")
-/*
- * The verifier counts one argument register for the __int128 and marks only
- * R1 through R3 at the entry of take_i128_global(), while the compiler passed
- * a in R1, v in R2:R3 and c in R4.
- */
-__failure __msg("R4 !read_ok")
+__success __retval(0)
 int aggregate_arg_int128_c_test(struct __sk_buff *skb)
 {
 	__u64 a = skb->len ^ MIX_A;
-- 
2.53.0-Meta

[PATCH bpf-next 05/12] bpf: Support by-value struct and __int128 kfunc arguments

[Yonghong Song]

A kfunc taking a struct or union by value is rejected today, and one
taking an __int128 is accepted but mis-counted:

  Unrecognized R2 type STRUCT

Give the kfunc argument walk the same slot count a BPF-to-BPF call now
uses: one argument register per eightbyte, so a 16-byte value takes two.
The outgoing stack argument count at the call site follows the slots for
the same reason. Similar to BPF-to-BPF aggregate handling, a kfunc
aggregate argument only supports tbe composed of scalars.

Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
---
 include/linux/filter.h |   1 +
 kernel/bpf/core.c      |  11 +++
 kernel/bpf/verifier.c  | 168 +++++++++++++++++++++++++++++++++++++----
 3 files changed, 164 insertions(+), 16 deletions(-)

diff --git a/include/linux/filter.h b/include/linux/filter.h
index 6e746b0a0930..e288df4124d3 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h
@@ -1238,6 +1238,7 @@ bool bpf_jit_supports_subprog_tailcalls(void);
 bool bpf_jit_supports_percpu_insn(void);
 bool bpf_jit_supports_kfunc_call(void);
 bool bpf_jit_supports_kfunc_ret_reg_pair(void);
+bool bpf_jit_supports_kfunc_arg_slot(u32 slots_used, u32 nslots, u32 align);
 bool bpf_jit_supports_stack_args(void);
 bool bpf_jit_supports_arena_args(void);
 bool bpf_jit_supports_far_kfunc_call(void);
diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
index 5db77d7915df..5a20d3e1ace9 100644
--- a/kernel/bpf/core.c
+++ b/kernel/bpf/core.c
@@ -3292,6 +3292,17 @@ bool __weak bpf_jit_supports_kfunc_ret_reg_pair(void)
 	return false;
 }
 
+/*
+ * Whether an argument of @nslots eightbytes and @align alignment, coming after
+ * @slots_used slots, lands where the kernel calling convention expects it. The
+ * JIT maps slots to argument positions in order, registers first, so the two
+ * agree unless the calling convention places the argument elsewhere.
+ */
+bool __weak bpf_jit_supports_kfunc_arg_slot(u32 slots_used, u32 nslots, u32 align)
+{
+	return false;
+}
+
 bool __weak bpf_jit_supports_stack_args(void)
 {
 	return false;
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 17e576c7b757..030fb37c22ef 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -12049,12 +12049,63 @@ bool bpf_is_kfunc_pkt_changing(struct bpf_call_arg_meta *meta)
 	return meta->func_id == special_kfunc_list[KF_bpf_xdp_pull_data];
 }
 
+static u32 kfunc_arg_slots(const struct btf_type *t)
+{
+	if (btf_type_is_int(t) || btf_type_is_struct(t))
+		return (t->size + BPF_REG_SIZE - 1) / BPF_REG_SIZE;
+	return 1;
+}
+
+static u32 kfunc_proto_slots(const struct btf *btf, const struct btf_type *func_proto)
+{
+	const struct btf_param *args = btf_params(func_proto);
+	u32 i, nargs = btf_type_vlen(func_proto), slots_used = 0;
+
+	for (i = 0; i < nargs; i++)
+		slots_used += kfunc_arg_slots(btf_type_skip_modifiers(btf, args[i].type, NULL));
+
+	return slots_used;
+}
+
+/*
+ * The alignment the calling convention gives a kfunc parameter of type @t, or
+ * 0 for a type that nests deeper than the walk descends. Only a 128-bit
+ * integer, or an aggregate built around one, asks for more than a register,
+ * and some conventions place such an argument differently.
+ */
+static u32 kfunc_arg_align(const struct btf *btf, const struct btf_type *t, int rec)
+{
+	const struct btf_member *member;
+	const struct btf_type *mt;
+	u32 align, i;
+
+	while (btf_type_is_array(t))
+		t = btf_type_skip_modifiers(btf, btf_array(t)->type, NULL);
+
+	if (btf_type_is_int(t))
+		return t->size > BPF_REG_SIZE ? t->size : BPF_REG_SIZE;
+	if (!btf_type_is_struct(t))
+		return BPF_REG_SIZE;
+	if (rec >= BTF_MEMBER_MAX_DEPTH)
+		return 0;
+
+	for_each_member(i, t, member) {
+		mt = btf_type_skip_modifiers(btf, member->type, NULL);
+		align = kfunc_arg_align(btf, mt, rec + 1);
+		if (!align)
+			return 0;
+		if (align > BPF_REG_SIZE)
+			return 2 * BPF_REG_SIZE;
+	}
+	return BPF_REG_SIZE;
+}
+
 static int
 get_kfunc_arg_type(struct bpf_verifier_env *env, struct bpf_call_arg_meta *meta,
-		   const struct btf_param *args, int arg, int nargs)
+		   const struct btf_param *args, int arg, int nargs, u32 slot)
 {
 	const struct btf_type *t, *ref_t = NULL;
-	argno_t argno = argno_from_arg(arg + 1);
+	argno_t argno = argno_from_arg(slot + 1);
 	const char *ref_tname = NULL;
 	int arg_type;
 
@@ -12062,6 +12113,14 @@ get_kfunc_arg_type(struct bpf_verifier_env *env, struct bpf_call_arg_meta *meta,
 
 	/* Scalar arguments are classified from their BTF suffix/name alone. */
 	if (btf_type_is_scalar(t)) {
+		if (t->size > 2 * BPF_REG_SIZE) {
+			verbose(env,
+				"%s type %s has size %u, only 1 to %d bytes "
+				"can be passed by value\n",
+				reg_arg_name(env, argno), btf_type_str(t), t->size,
+				2 * BPF_REG_SIZE);
+			return -EINVAL;
+		}
 		if (is_kfunc_arg_constant(meta->btf, &args[arg]))
 			return KF_ARG_CONST;
 		if (is_kfunc_arg_const_mem_size(meta->btf, &args[arg]))
@@ -12074,6 +12133,23 @@ get_kfunc_arg_type(struct bpf_verifier_env *env, struct bpf_call_arg_meta *meta,
 		return KF_ARG_ANYTHING;
 	}
 
+	if (btf_type_is_struct(t)) {
+		if (!t->size || t->size > 2 * BPF_REG_SIZE) {
+			verbose(env,
+				"%s type %s has size %u, only 1 to %d bytes "
+				"can be passed by value\n",
+				reg_arg_name(env, argno), btf_type_str(t), t->size,
+				2 * BPF_REG_SIZE);
+			return -EINVAL;
+		}
+		if (!btf_type_is_scalar_struct(env, meta->btf, t)) {
+			verbose(env, "%s type %s is not composed of scalars\n",
+				reg_arg_name(env, argno), btf_type_str(t));
+			return -EINVAL;
+		}
+		return KF_ARG_ANYTHING;
+	}
+
 	if (!btf_type_is_ptr(t)) {
 		verbose(env, "Unrecognized %s type %s\n",
 			reg_arg_name(env, argno), btf_type_str(t));
@@ -12189,7 +12265,7 @@ static int gen_kfunc_arg_proto(struct bpf_verifier_env *env, struct bpf_call_arg
 {
 	const struct btf *btf = meta->btf;
 	const struct btf_param *args;
-	u32 i, nargs;
+	u32 i, nargs, slots_used;
 	int arg_type;
 
 	args = (const struct btf_param *)(meta->func_proto + 1);
@@ -12205,19 +12281,55 @@ static int gen_kfunc_arg_proto(struct bpf_verifier_env *env, struct bpf_call_arg
 		return -ENOTSUPP;
 	}
 
-	for (i = 0; i < nargs; i++) {
+	for (i = 0, slots_used = 0; i < nargs; i++) {
+		const struct btf_type *t;
+		u32 align, nslots;
+
+		t = btf_type_skip_modifiers(btf, args[i].type, NULL);
+		nslots = kfunc_arg_slots(t);
+		if (nslots > 1) {
+			align = kfunc_arg_align(btf, t, 0);
+			if (!align) {
+				verbose(env,
+					"Function %s arg#%d type %s nests structs more than "
+					"%d levels deep\n",
+					meta->func_name, i, btf_type_str(t),
+					BTF_MEMBER_MAX_DEPTH);
+				return -EINVAL;
+			}
+			if (!bpf_jit_supports_kfunc_arg_slot(slots_used, nslots, align)) {
+				verbose(env,
+					"Function %s arg#%d type %s cannot be passed at "
+					"argument slot %d on this architecture\n",
+					meta->func_name, i, btf_type_str(t), slots_used);
+				return -EINVAL;
+			}
+		}
+		slots_used += nslots;
+
 		if (is_kfunc_arg_prog_aux(btf, &args[i]) ||
 		    is_kfunc_arg_ignore(btf, &args[i]) ||
 		    is_kfunc_arg_implicit(meta, i))
 			continue;
 
-		arg_type = get_kfunc_arg_type(env, meta, args, i, nargs);
+		arg_type = get_kfunc_arg_type(env, meta, args, i, nargs, slots_used - nslots);
 		if (arg_type < 0)
 			return arg_type;
 
 		proto->arg_type[i] = arg_type;
 	}
 
+	if (slots_used > MAX_BPF_FUNC_ARGS) {
+		verbose(env, "Function %s needs %d > %d argument slots\n", meta->func_name,
+			slots_used, MAX_BPF_FUNC_ARGS);
+		return -EINVAL;
+	}
+	if (slots_used > MAX_BPF_FUNC_REG_ARGS && !bpf_jit_supports_stack_args()) {
+		verbose(env, "JIT does not support kfunc %s() with %d argument slots\n",
+			meta->func_name, slots_used);
+		return -ENOTSUPP;
+	}
+
 	return 0;
 }
 
@@ -12792,29 +12904,34 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_call_arg_me
 	const struct btf *btf = meta->btf;
 	const struct btf_param *args;
 	struct btf_record *rec;
-	u32 i, nargs;
+	u32 i, k, nargs, proto_slots, slots_used, nslots = 0;
 	int ret;
 
 	args = (const struct btf_param *)(meta->func_proto + 1);
 	nargs = btf_type_vlen(meta->func_proto);
+	proto_slots = kfunc_proto_slots(btf, meta->func_proto);
 
-	ret = check_outgoing_stack_args(env, caller, nargs, func_name, btf, args);
+	ret = check_outgoing_stack_args(env, caller, proto_slots, func_name, btf,
+					proto_slots == nargs ? args : NULL);
 	if (ret)
 		return ret;
 
 	/* Check that BTF function arguments match actual types that the
 	 * verifier sees.
 	 */
-	for (i = 0; i < nargs; i++) {
-		struct bpf_reg_state *reg = get_func_arg_reg(caller, regs, i);
+	for (i = 0, slots_used = 0; i < nargs; i++, slots_used += nslots) {
+		struct bpf_reg_state *reg = get_func_arg_reg(caller, regs, slots_used);
 		const struct btf_type *t, *ref_t, *resolve_ret;
 		enum bpf_arg_type arg_type = ARG_DONTCARE;
-		argno_t argno = argno_from_arg(i + 1);
+		argno_t argno = argno_from_arg(slots_used + 1);
 		int regno = reg_from_argno(argno);
 		bool btf_id_fixed_off_ok = true;
 		u32 ref_id = args[i].type, type_size;
 		int kf_arg_type = meta->fn->arg_type[i];
 
+		t = btf_type_skip_modifiers(btf, args[i].type, NULL);
+		nslots = kfunc_arg_slots(t);
+
 		if (is_kfunc_arg_prog_aux(btf, &args[i])) {
 			/* Reject repeated use bpf_prog_aux */
 			if (meta->arg_prog) {
@@ -12834,8 +12951,6 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_call_arg_me
 		if (is_kfunc_arg_ignore(btf, &args[i]) || is_kfunc_arg_implicit(meta, i))
 			continue;
 
-		t = btf_type_skip_modifiers(btf, args[i].type, NULL);
-
 		if (btf_type_is_ptr(t)) {
 			ref_t = btf_type_skip_modifiers(btf, t->type, &ref_id);
 			ref_tname = btf_name_by_offset(btf, ref_t->name_off);
@@ -12882,6 +12997,27 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_call_arg_me
 			ref_tname = btf_name_by_offset(btf, ref_t->name_off);
 		}
 
+		/*
+		 * The first register is checked in below KF_ARG_ANYTHING.
+		 * The rest of it has to be a scalar.
+		 */
+		for (k = 1; k < nslots; k++) {
+			argno_t hi_argno = argno_from_arg(slots_used + k + 1);
+			struct bpf_reg_state *hi = get_func_arg_reg(caller, regs, slots_used + k);
+
+			if (hi->type != SCALAR_VALUE) {
+				verbose(env, "%s is not a scalar\n", reg_arg_name(env, hi_argno));
+				bpf_diag_call_arg_fmt(env, insn_idx, hi_argno, func_name,
+						      "Pass an integer scalar value for this "
+						      "argument, not a pointer or resource object.",
+						      "the kfunc expects an integer scalar, "
+						      "but %s is %s",
+						      reg_arg_name(env, hi_argno),
+						      bpf_diag_reg_type_plain(env, hi->type));
+				return -EINVAL;
+			}
+		}
+
 		switch (base_type(kf_arg_type)) {
 		case KF_ARG_CONST:
 		case KF_ARG_CONST_MEM_SIZE:
@@ -13882,7 +14018,7 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
 	struct bpf_insn_aux_data *insn_aux;
 	const char *operation;
 	int err, insn_idx = *insn_idx_p;
-	u32 i, nargs, ptr_type_id, ret_nregs = 1;
+	u32 i, proto_slots, ptr_type_id, ret_nregs = 1;
 	struct bpf_kfunc_desc *desc;
 	struct btf *desc_btf;
 	int id;
@@ -14308,11 +14444,11 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
 	if (bpf_is_kfunc_pkt_changing(&meta))
 		clear_all_pkt_pointers(env);
 
-	nargs = btf_type_vlen(meta.func_proto);
-	if (nargs > MAX_BPF_FUNC_REG_ARGS) {
+	proto_slots = kfunc_proto_slots(desc_btf, meta.func_proto);
+	if (proto_slots > MAX_BPF_FUNC_REG_ARGS) {
 		struct bpf_func_state *caller = cur_func(env);
 		struct bpf_subprog_info *caller_info = &env->subprog_info[caller->subprogno];
-		u16 out_stack_arg_cnt = nargs - MAX_BPF_FUNC_REG_ARGS;
+		u16 out_stack_arg_cnt = proto_slots - MAX_BPF_FUNC_REG_ARGS;
 		u16 stack_arg_cnt = bpf_in_stack_arg_cnt(caller_info) + out_stack_arg_cnt;
 
 		if (stack_arg_cnt > caller_info->stack_arg_cnt)
-- 
2.53.0-Meta

[PATCH bpf-next 06/12] bpf: Add a JIT helper for the outgoing stack of kfunc calls

[Yonghong Song]

Add bpf_jit_kfunc_stack_slots() helper which will be used
in x86 and arm64 bpf jit.

Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
---
 include/linux/filter.h |  2 ++
 kernel/bpf/core.c      | 31 +++++++++++++++++++++++++++++++
 2 files changed, 33 insertions(+)

diff --git a/include/linux/filter.h b/include/linux/filter.h
index e288df4124d3..eddcb9987056 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h
@@ -1239,6 +1239,8 @@ bool bpf_jit_supports_percpu_insn(void);
 bool bpf_jit_supports_kfunc_call(void);
 bool bpf_jit_supports_kfunc_ret_reg_pair(void);
 bool bpf_jit_supports_kfunc_arg_slot(u32 slots_used, u32 nslots, u32 align);
+u16 bpf_jit_kfunc_stack_slots(const struct bpf_prog *prog, u32 nr_regs,
+			      int (*layout)(const struct btf_func_model *fm, u8 *pos, int max));
 bool bpf_jit_supports_stack_args(void);
 bool bpf_jit_supports_arena_args(void);
 bool bpf_jit_supports_far_kfunc_call(void);
diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
index 5a20d3e1ace9..8fea01377298 100644
--- a/kernel/bpf/core.c
+++ b/kernel/bpf/core.c
@@ -3303,6 +3303,37 @@ bool __weak bpf_jit_supports_kfunc_arg_slot(u32 slots_used, u32 nslots, u32 alig
 	return false;
 }
 
+/*
+ * The most outgoing stack eightbytes any kfunc call in @prog needs.
+ * @layout writes a position per eightbyte and returns how many, or an error.
+ * @nr_regs is how many of those positions are argument registers.
+ */
+u16 bpf_jit_kfunc_stack_slots(const struct bpf_prog *prog, u32 nr_regs,
+			      int (*layout)(const struct btf_func_model *fm, u8 *pos, int max))
+{
+	const struct bpf_insn *insn = prog->insnsi;
+	u8 pos[MAX_BPF_FUNC_ARGS];
+	int i, k, n, slots = 0;
+
+	for (i = 0; i < prog->len; i++, insn++) {
+		const struct btf_func_model *fm;
+
+		if (insn->code != (BPF_JMP | BPF_CALL) ||
+		    insn->src_reg != BPF_PSEUDO_KFUNC_CALL)
+			continue;
+		fm = bpf_jit_find_kfunc_model(prog, insn);
+		if (!fm)
+			continue;
+		n = layout(fm, pos, ARRAY_SIZE(pos));
+		if (n < 0)
+			continue;
+		for (k = 0; k < n; k++)
+			if (pos[k] >= nr_regs)
+				slots = max(slots, pos[k] - (int)nr_regs + 1);
+	}
+	return slots;
+}
+
 bool __weak bpf_jit_supports_stack_args(void)
 {
 	return false;
-- 
2.53.0-Meta

[PATCH bpf-next 07/12] bpf, x86: Place kfunc arguments per the SysV calling convention

[Yonghong Song]

The JIT hands each eightbyte the BPF calling convention passes an
argument in to the argument position of the same number, registers first,
so the two conventions agree unless the kernel one places an argument
somewhere else. Compute where SysV wants each eightbyte, and move the
ones that differ before the call.

SysV disagrees over an argument that the registers left cannot hold: it
moves the whole of it to the stack and leaves the registers to the
arguments that follow, while the BPF convention splits it and keeps
filling slots in order. So for

  u64 f(u64 a, u64 b, u64 c, u64 d, u64 e, struct pair s);

the BPF convention puts s in the last argument register and the first
stack slot, while SysV puts it wholly on the stack. Add an argument after
s and it takes the register s vacated, which makes the moves a cycle, so
one value at a time waits in RAX, dead before a call.

The outgoing argument area is sized for both conventions, as SysV can put
on the stack an argument the BPF slots kept in a register, and
bpf_jit_supports_kfunc_arg_slot() can now answer yes to any placement.

In addition, the arena argument walk counts eightbytes rather than
parameters, as an argument may take two registers.

Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
---
 arch/x86/net/bpf_jit_comp.c | 185 +++++++++++++++++++++++++++++++++++-
 1 file changed, 182 insertions(+), 3 deletions(-)

diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
index 48429fae0641..0b07320ad011 100644
--- a/arch/x86/net/bpf_jit_comp.c
+++ b/arch/x86/net/bpf_jit_comp.c
@@ -1682,6 +1682,164 @@ static int emit_spectre_bhb_barrier(u8 **pprog, u8 *ip,
 	return 0;
 }
 
+/* The kernel ABI hands the first six eightbytes of arguments to registers. */
+static const u32 x86_arg_reg[6] = {
+	BPF_REG_1, BPF_REG_2, BPF_REG_3, BPF_REG_4, BPF_REG_5, X86_REG_R9,
+};
+
+/* Fill argument positions based on the kernel calling convention. */
+static int kfunc_arg_layout(const struct btf_func_model *fm, u8 *pos, int max)
+{
+	int i, k, nregs = 0, nstack = 0, slot = 0;
+
+	for (i = 0; i < fm->nr_args; i++) {
+		int n = (fm->arg_size[i] + 7) / 8;
+
+		if (slot + n > max)
+			return -EINVAL;
+		if (nregs + n <= 6)
+			for (k = 0; k < n; k++)
+				pos[slot++] = nregs++;
+		else
+			for (k = 0; k < n; k++)
+				pos[slot++] = 6 + nstack++;
+	}
+	return slot;
+}
+
+static u16 kfunc_arg_stack_bytes(const struct bpf_prog *prog)
+{
+	return bpf_jit_kfunc_stack_slots(prog, 6, kfunc_arg_layout) * 8;
+}
+
+static void emit_arg_pos_load(u8 **pprog, u32 reg, u8 pos, s32 stack_base)
+{
+	if (pos < 6)
+		emit_mov_reg(pprog, true, reg, x86_arg_reg[pos]);
+	else
+		emit_ldx(pprog, BPF_DW, reg, BPF_REG_FP,
+			 stack_base + (pos - 6) * 8);
+}
+
+static void emit_arg_pos_store(u8 **pprog, u8 pos, u32 reg, s32 stack_base)
+{
+	if (pos < 6)
+		emit_mov_reg(pprog, true, x86_arg_reg[pos], reg);
+	else
+		emit_stx(pprog, BPF_DW, BPF_REG_FP, reg,
+			 stack_base + (pos - 6) * 8);
+}
+
+static void emit_arg_pos_move(u8 **pprog, u8 to, u8 from, s32 stack_base)
+{
+	if (from < 6) {
+		emit_arg_pos_store(pprog, to, x86_arg_reg[from], stack_base);
+		return;
+	}
+	if (to < 6) {
+		emit_arg_pos_load(pprog, x86_arg_reg[to], from, stack_base);
+		return;
+	}
+	emit_arg_pos_load(pprog, AUX_REG, from, stack_base);
+	emit_arg_pos_store(pprog, to, AUX_REG, stack_base);
+}
+
+/*
+ * Put the arguments of a kfunc call where the kernel ABI expects them, given
+ * that the BPF ABI has already put them in its own slots. Returns the number
+ * of emitted bytes, or a negative error.
+ *
+ * This is the parallel move problem: emit every move whose destination no
+ * longer holds a value, then break each remaining cycle with one temporary.
+ * See Rideau, Serpette and Leroy, "Tilting at Windmills with Coq: Formal
+ * Verification of a Compilation Algorithm for Parallel Moves", Journal of
+ * Automated Reasoning 45(2), 2010.
+ *
+ * For
+ *
+ *   u64 f(u64 a, u64 b, u64 c, u64 d, u64 e, struct pair s);
+ *
+ *     slot     0 1 2 3 4 5 6
+ *     position 0 1 2 3 4 6 7
+ *     final    0 1 2 3 4 ? 6 7
+ *
+ * the BPF ABI splits s between the last register and the stack while the
+ * kernel one takes it wholly on the stack, so each of its eightbytes moves up
+ * one position, the last one first, and position 5 (R9) is left unused.
+ * Adding an argument after s,
+ *
+ *   u64 g(u64 a, u64 b, u64 c, u64 d, u64 e, struct pair s, u64 f);
+ *
+ *     slot     0 1 2 3 4 5 6 7
+ *     position 0 1 2 3 4 6 7 5
+ *     final    0 1 2 3 4 5 6 7
+ *
+ * gives f the register s vacated, and 5 -> 6 -> 7 -> 5 is a cycle: slot 5
+ * waits in PARK_REG while slots 7 and 6 move, and is stored last. AUX_REG
+ * carries a value between two stack positions and cannot be the one that
+ * waits, while RAX is dead before a call, being where the return value
+ * arrives.
+ */
+#define PARK_REG BPF_REG_0
+
+static int emit_kfunc_args(const struct btf_func_model *fm, u8 **pprog,
+			   s32 stack_base)
+{
+	u8 *prog = *pprog, *start = prog;
+	bool done[MAX_BPF_FUNC_ARGS] = {};
+	u8 pos[MAX_BPF_FUNC_ARGS];
+	int i, j, n, todo = 0, parked = -1;
+
+	n = kfunc_arg_layout(fm, pos, ARRAY_SIZE(pos));
+	if (n < 0)
+		return 0;
+	for (i = 0; i < n; i++)
+		if (pos[i] != i)
+			todo++;
+	if (!todo)
+		return 0;
+
+	while (todo) {
+		bool moved = false;
+
+		for (i = 0; i < n; i++) {
+			if (done[i] || pos[i] == i)
+				continue;
+			/* Writing there would lose a value still to be moved. */
+			for (j = 0; j < n; j++)
+				if (!done[j] && j != parked && pos[j] != j && j == pos[i])
+					break;
+			if (j < n)
+				continue;
+			if (i == parked) {
+				emit_arg_pos_store(&prog, pos[i], PARK_REG, stack_base);
+				parked = -1;
+			} else {
+				emit_arg_pos_move(&prog, pos[i], i, stack_base);
+			}
+			done[i] = true;
+			todo--;
+			moved = true;
+		}
+		if (moved)
+			continue;
+
+		/* Every move left would clobber a value: break a cycle. */
+		if (parked >= 0)
+			return -EFAULT;
+		for (i = 0; i < n; i++)
+			if (!done[i] && pos[i] != i)
+				break;
+		if (i == n)
+			return -EFAULT;
+		emit_arg_pos_load(&prog, PARK_REG, i, stack_base);
+		parked = i;
+	}
+
+	*pprog = prog;
+	return prog - start;
+}
+
 /*
  * Rebase the __arena args of a kfunc call to arena kernel addresses,
  * rN = kern_vm_start + (u32)rN, with R12 holding kern_vm_start. A nullable
@@ -1693,11 +1851,21 @@ static int emit_kfunc_arena_args(struct bpf_prog *bpf_prog,
 {
 	u8 *prog = *pprog;
 	u8 *start = prog;
-	int i;
+	int i, slot;
 
-	for (i = 0; i < min_t(int, fm->nr_args, MAX_BPF_FUNC_REG_ARGS); i++) {
+	for (i = 0, slot = 0; i < fm->nr_args; i++) {
+		u32 arg_regs = (fm->arg_size[i] + 7) / 8;
 		u8 flags = fm->arg_flags[i];
-		u32 reg = BPF_REG_1 + i;
+		u32 reg;
+
+		if (slot + arg_regs > MAX_BPF_FUNC_REG_ARGS) {
+			/* The verifier refuses an arena pointer past the registers. */
+			if (WARN_ON_ONCE(flags & BTF_FMODEL_ARENA_ARG))
+				return -EFAULT;
+			break;
+		}
+		reg = BPF_REG_1 + slot;
+		slot += arg_regs;
 
 		if (!(flags & BTF_FMODEL_ARENA_ARG))
 			continue;
@@ -1832,6 +2000,7 @@ static int do_jit(struct bpf_verifier_env *env, struct bpf_prog *bpf_prog, int *
 	 * Arg 6 goes into r9 register, not on stack.
 	 */
 	outgoing_rsp = out_stack_arg_cnt > 1 ? (out_stack_arg_cnt - 1) * 8 : 0;
+	outgoing_rsp = max(outgoing_rsp, kfunc_arg_stack_bytes(bpf_prog));
 	if (bpf_prog->aux->exception_boundary)
 		bpf_prog->aux->stack_arg_sp_adjust = outgoing_rsp;
 	emit_sub_rsp(&prog, outgoing_rsp);
@@ -2656,6 +2825,11 @@ st:			insn_off = insn->off;
 				if (err < 0)
 					return err;
 				ip += err;
+				err = emit_kfunc_args(fm, &prog,
+						      outgoing_arg_base - outgoing_rsp);
+				if (err < 0)
+					return err;
+				ip += err;
 			}
 			if (priv_frame_ptr) {
 				push_r9(&prog);
@@ -4169,6 +4343,11 @@ bool bpf_jit_supports_kfunc_ret_reg_pair(void)
 	return true;
 }
 
+bool bpf_jit_supports_kfunc_arg_slot(u32 slots_used, u32 nslots, u32 align)
+{
+	return true;
+}
+
 bool bpf_jit_supports_stack_args(void)
 {
 	return true;
-- 
2.53.0-Meta

[PATCH bpf-next 08/12] bpf: Record a 16-byte argument alignment in the function model

[Yonghong Song]

A calling convention may place an argument aligned to 16 bytes apart from
the rest: AAPCS64 rounds the register number up to an even one for it. The
function model a JIT works from carries the size of every argument and a
few flags, but not its alignment, so a JIT cannot tell such an argument
from any other one of the same size.

Have btf_distill_func_proto() record it as BTF_FMODEL_ALIGN16_ARG. Only a
128-bit integer, or an aggregate built around one, asks for the alignment,
which btf_type_align16() answers by walking the members.

The walk descends a bounded number of levels, and a type nested deeper has
no answer. Reporting "not aligned" there would have a JIT place the
argument in a register the kernel does not read, so the model is refused
instead and the call is never compiled.

The flag will be used in the next patch.

Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
---
 include/linux/bpf.h          |  3 +++
 include/linux/bpf_verifier.h |  1 +
 kernel/bpf/btf.c             | 11 ++++++++++-
 kernel/bpf/verifier.c        | 25 +++++++++++++++++++++++++
 4 files changed, 39 insertions(+), 1 deletion(-)

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 3a7eb2185c35..5b93aa97f1bc 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -1193,6 +1193,9 @@ struct bpf_prog_offload {
 	u32			jited_len;
 };
 
+/* Argument aligned to 16 bytes. */
+#define BTF_FMODEL_ALIGN16_ARG		BIT(0)
+
 /* The argument is signed. */
 #define BTF_FMODEL_SIGNED_ARG		BIT(1)
 
diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
index ae9f606539f4..2c51b0f8b18c 100644
--- a/include/linux/bpf_verifier.h
+++ b/include/linux/bpf_verifier.h
@@ -1497,6 +1497,7 @@ enum btf_member_kind {
 
 bool btf_struct_is_composed_of(struct bpf_verifier_env *env, const struct btf *btf,
 			       const struct btf_type *t, u32 member_kinds);
+int btf_type_align16(const struct btf *btf, const struct btf_type *t, int rec);
 
 int bpf_find_subprog(struct bpf_verifier_env *env, int off);
 bool bpf_is_throw_kfunc(struct bpf_insn *insn);
diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index 6c391449e298..f933a675b0a8 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -7576,7 +7576,7 @@ int btf_distill_func_proto(struct bpf_verifier_log *log,
 	const struct btf_param *args;
 	const struct btf_type *t;
 	u32 i, nargs;
-	int ret;
+	int align16, ret;
 
 	if (!func) {
 		/* BTF function prototype doesn't match the verifier types.
@@ -7633,6 +7633,15 @@ int btf_distill_func_proto(struct bpf_verifier_log *log,
 		}
 		m->arg_size[i] = ret;
 		m->arg_flags[i] = __get_arg_fmodel_flags(btf, &args[i], t);
+
+		align16 = btf_type_align16(btf, t, 0);
+		if (align16 < 0) {
+			bpf_log(log, "The function %s arg%d type %s has unknown alignment.\n",
+				tname, i, btf_type_str(t));
+			return -EINVAL;
+		} else if (align16) {
+			m->arg_flags[i] |= BTF_FMODEL_ALIGN16_ARG;
+		}
 	}
 	m->nr_args = nargs;
 	return 0;
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 030fb37c22ef..461883158ffa 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -11748,6 +11748,31 @@ bool btf_struct_is_composed_of(struct bpf_verifier_env *env,
 	return btf_struct_member_walk(env, btf, t, member_kinds, 0, NULL);
 }
 
+int btf_type_align16(const struct btf *btf, const struct btf_type *t, int rec)
+{
+	const struct btf_member *member;
+	int ret;
+	u32 i;
+
+	while (btf_type_is_array(t))
+		t = btf_type_skip_modifiers(btf, btf_array(t)->type, NULL);
+
+	if (btf_type_is_int(t))
+		return t->size > 8;
+	if (!btf_type_is_struct(t))
+		return 0;
+	if (rec >= BTF_MEMBER_MAX_DEPTH)
+		return -E2BIG;
+
+	for_each_member(i, t, member) {
+		ret = btf_type_align16(btf, btf_type_skip_modifiers(btf, member->type, NULL),
+				       rec + 1);
+		if (ret)
+			return ret;
+	}
+	return 0;
+}
+
 static bool btf_type_is_scalar_struct(struct bpf_verifier_env *env,
 				      const struct btf *btf,
 				      const struct btf_type *t)
-- 
2.53.0-Meta

[PATCH bpf-next 09/12] bpf, arm64: Place kfunc arguments per AAPCS64

[Yonghong Song]

As on x86-64, the JIT hands each eightbyte the BPF calling convention
passes an argument in to the argument position of the same number, and
moves the ones AAPCS64 wants elsewhere.

AAPCS64 has eight argument registers and disagrees in two ways. It rounds
the register number up to an even one for an argument aligned to 16
bytes, so

  u64 f(u64 a, __int128 v, u64 b);

wants v in x2 and x3 where the BPF convention put it in x1 and x2. And it
gives no register to anything once an argument has gone to the stack,
which the BPF convention, with three argument registers fewer, reaches
sooner. Both only ever move an eightbyte further along than the BPF
convention put it, so moving the last one first is enough and no value
has to wait anywhere.

The alignment reaches the JIT as BTF_FMODEL_ALIGN16_ARG in the function
model.

An argument AAPCS64 places on the stack while the BPF convention kept it
in a register needs room the BPF slots do not account for, so the
outgoing argument area is sized for both. As on x86-64, an eightbyte
whose position equals its slot needs no move,
bpf_jit_supports_kfunc_arg_slot() can now answer yes to any placement,
and the arena argument walk counts eightbytes rather than parameters.

Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
---
 arch/arm64/net/bpf_jit_comp.c | 112 +++++++++++++++++++++++++++++++++-
 1 file changed, 109 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/net/bpf_jit_comp.c b/arch/arm64/net/bpf_jit_comp.c
index 3aa3ea0bc30b..f6c783d176ed 100644
--- a/arch/arm64/net/bpf_jit_comp.c
+++ b/arch/arm64/net/bpf_jit_comp.c
@@ -1256,6 +1256,93 @@ static void emit_stack_arg_store_imm(s32 imm, s16 bpf_off, const u8 tmp, struct
 	}
 }
 
+/* The kernel ABI hands the first eight eightbytes of arguments to registers. */
+static const u8 a64_arg_reg[8] = {
+	A64_R(0), A64_R(1), A64_R(2), A64_R(3),
+	A64_R(4), A64_R(5), A64_R(6), A64_R(7),
+};
+
+static int kfunc_arg_layout(const struct btf_func_model *fm, u8 *pos, int max)
+{
+	int i, k, ngrn = 0, nsaa = 0, slot = 0;
+
+	for (i = 0; i < fm->nr_args; i++) {
+		bool align16 = fm->arg_flags[i] & BTF_FMODEL_ALIGN16_ARG;
+		int n = (fm->arg_size[i] + 7) / 8;
+
+		if (slot + n > max)
+			return -EINVAL;
+		if (align16)
+			ngrn = round_up(ngrn, 2);
+		if (ngrn + n <= 8) {
+			for (k = 0; k < n; k++)
+				pos[slot++] = ngrn++;
+			continue;
+		}
+		/* Nothing that follows gets a register either. */
+		ngrn = 8;
+		if (align16)
+			nsaa = round_up(nsaa, 2);
+		for (k = 0; k < n; k++)
+			pos[slot++] = 8 + nsaa++;
+	}
+	return slot;
+}
+
+static u16 kfunc_arg_stack_bytes(const struct bpf_prog *prog)
+{
+	u16 slots = bpf_jit_kfunc_stack_slots(prog, 8, kfunc_arg_layout);
+
+	return round_up(slots * sizeof(u64), 16);
+}
+
+static void emit_arg_pos_load(u8 reg, u8 pos, struct jit_ctx *ctx)
+{
+	if (pos < 8)
+		emit(A64_MOV(1, reg, a64_arg_reg[pos]), ctx);
+	else
+		emit(A64_LDR64I(reg, A64_SP, (pos - 8) * sizeof(u64)), ctx);
+}
+
+static void emit_arg_pos_store(u8 pos, u8 reg, struct jit_ctx *ctx)
+{
+	if (pos < 8)
+		emit(A64_MOV(1, a64_arg_reg[pos], reg), ctx);
+	else
+		emit(A64_STR64I(reg, A64_SP, (pos - 8) * sizeof(u64)), ctx);
+}
+
+static int emit_kfunc_args(const struct bpf_insn *insn, struct jit_ctx *ctx)
+{
+	const u8 tmp = bpf2a64[TMP_REG_1];
+	const struct btf_func_model *fm;
+	u8 pos[MAX_BPF_FUNC_ARGS];
+	int i, n;
+
+	fm = bpf_jit_find_kfunc_model(ctx->prog, insn);
+	if (!fm)
+		return -EINVAL;
+
+	n = kfunc_arg_layout(fm, pos, ARRAY_SIZE(pos));
+	if (n < 0)
+		return 0;
+
+	for (i = n - 1; i >= 0; i--) {
+		if (pos[i] == i)
+			continue;
+		if (WARN_ON_ONCE(pos[i] < i))
+			return -EFAULT;
+		if (pos[i] < 8) {
+			/* into a register, read straight from the slot */
+			emit_arg_pos_load(a64_arg_reg[pos[i]], i, ctx);
+		} else {
+			emit_arg_pos_load(tmp, i, ctx);
+			emit_arg_pos_store(pos[i], tmp, ctx);
+		}
+	}
+	return 0;
+}
+
 /*
  * Rebase the __arena args of a kfunc call to arena kernel addresses,
  * xN = kern_vm_start + (u32)xN, with the arena base register holding
@@ -1266,15 +1353,25 @@ static int emit_kfunc_arena_args(struct jit_ctx *ctx, const struct bpf_insn *ins
 {
 	const u8 arena_vm_base = bpf2a64[ARENA_VM_START];
 	const struct btf_func_model *fm;
-	int i;
+	int i, slot;
 
 	fm = bpf_jit_find_kfunc_model(ctx->prog, insn);
 	if (!fm)
 		return -EINVAL;
 
-	for (i = 0; i < min_t(int, fm->nr_args, MAX_BPF_FUNC_REG_ARGS); i++) {
-		const u8 reg = bpf2a64[BPF_REG_1 + i];
+	for (i = 0, slot = 0; i < fm->nr_args; i++) {
+		u32 arg_regs = (fm->arg_size[i] + 7) / 8;
 		u8 flags = fm->arg_flags[i];
+		u8 reg;
+
+		if (slot + arg_regs > MAX_BPF_FUNC_REG_ARGS) {
+			/* The verifier refuses an arena pointer past the registers. */
+			if (WARN_ON_ONCE(flags & BTF_FMODEL_ARENA_ARG))
+				return -EFAULT;
+			break;
+		}
+		reg = bpf2a64[BPF_REG_1 + slot];
+		slot += arg_regs;
 
 		if (!(flags & BTF_FMODEL_ARENA_ARG))
 			continue;
@@ -1719,6 +1816,9 @@ static int build_insn(const struct bpf_verifier_env *env, const struct bpf_insn
 			ret = emit_kfunc_arena_args(ctx, insn);
 			if (ret < 0)
 				return ret;
+			ret = emit_kfunc_args(insn, ctx);
+			if (ret < 0)
+				return ret;
 		}
 		emit_call(func_addr, ctx);
 		/*
@@ -2223,6 +2323,7 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_verifier_env *env, struct bpf_pr
 		if (nr_on_stack > 0)
 			ctx.stack_arg_size = round_up(nr_on_stack * sizeof(u64), 16);
 	}
+	ctx.stack_arg_size = max(ctx.stack_arg_size, kfunc_arg_stack_bytes(prog));
 
 	if (priv_stack_ptr)
 		ctx.priv_sp_used = true;
@@ -2393,6 +2494,11 @@ bool bpf_jit_supports_kfunc_ret_reg_pair(void)
 	return true;
 }
 
+bool bpf_jit_supports_kfunc_arg_slot(u32 slots_used, u32 nslots, u32 align)
+{
+	return true;
+}
+
 bool bpf_jit_supports_stack_args(void)
 {
 	return true;
-- 
2.53.0-Meta

[PATCH bpf-next 10/12] selftests/bpf: Add C tests for by-value arguments up to 16 bytes

[Yonghong Song]

Extend the by-value argument test with the aggregate cases, written in C
so that they depend on the compiler lowering the argument into a pair of
argument registers rather than on a hand-written register layout.

The programs cover a struct and a union that fill two registers, a
smaller struct that fills one, and two struct arguments in a row, each
with an int argument on either side so that a wrong slot count shows up
as a wrong value in the parameters around it, alongside the __int128
already there. A global function taking a struct with a pointer member
is rejected: the callee would receive the pointer as an opaque scalar.

Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
---
 .../selftests/bpf/progs/verifier_int128_arg.c | 164 ++++++++++++++++++
 1 file changed, 164 insertions(+)

diff --git a/tools/testing/selftests/bpf/progs/verifier_int128_arg.c b/tools/testing/selftests/bpf/progs/verifier_int128_arg.c
index 044fc4a80055..b10fb4aeb405 100644
--- a/tools/testing/selftests/bpf/progs/verifier_int128_arg.c
+++ b/tools/testing/selftests/bpf/progs/verifier_int128_arg.c
@@ -7,6 +7,131 @@
 #define MIX_A	0xdeadbeefcafef00dULL
 #define MIX_B	0x0123456789abcdefULL
 
+struct pair {
+	__u64 lo;
+	__u64 hi;
+};
+
+struct small {
+	__u32 a;
+	__u32 b;
+};
+
+union upair {
+	__u64 halves[2];
+	struct {
+		__u64 lo;
+		__u64 hi;
+	} parts;
+};
+
+struct with_ptr {
+	void *p;
+	__u64 x;
+};
+
+static __noinline __u64 take_pair(int a, struct pair p, int c)
+{
+	return (__u64)a + p.lo + p.hi + c;
+}
+
+SEC("tc")
+__success __retval(0)
+int aggregate_arg_static_struct_c_test(struct __sk_buff *skb)
+{
+	__u64 a = skb->len ^ MIX_A;
+	__u64 b = skb->len ^ MIX_B;
+	struct pair p = { .lo = a, .hi = b };
+
+	if (take_pair(1, p, 2) != a + b + 3)
+		return 1;
+
+	return 0;
+}
+
+#if defined(__clang__)
+
+__noinline __u64 take_pair_global(int a, struct pair p, int c)
+{
+	return (__u64)a + p.lo + p.hi + c;
+}
+
+SEC("tc")
+__success __retval(0)
+int aggregate_arg_global_struct_c_test(struct __sk_buff *skb)
+{
+	__u64 a = skb->len ^ MIX_A;
+	__u64 b = skb->len ^ MIX_B;
+	struct pair p = { .lo = a, .hi = b };
+
+	if (take_pair_global(1, p, 2) != a + b + 3)
+		return 1;
+
+	return 0;
+}
+
+__noinline __u64 take_two_pairs_global(struct pair p, struct pair q)
+{
+	return p.lo + p.hi + q.lo + q.hi;
+}
+
+SEC("tc")
+__success __retval(0)
+int aggregate_arg_two_structs_c_test(struct __sk_buff *skb)
+{
+	__u64 a = skb->len ^ MIX_A;
+	__u64 b = skb->len ^ MIX_B;
+	struct pair p = { .lo = a, .hi = b };
+	struct pair q = { .lo = b, .hi = a };
+
+	if (take_two_pairs_global(p, q) != 2 * (a + b))
+		return 1;
+
+	return 0;
+}
+
+__noinline __u64 take_small_global(int a, struct small s, int c)
+{
+	return (__u64)a + s.a + s.b + c;
+}
+
+SEC("tc")
+__success __retval(0)
+int aggregate_arg_small_struct_c_test(struct __sk_buff *skb)
+{
+	__u32 a = skb->len ^ (__u32)MIX_A;
+	__u32 b = skb->len ^ (__u32)MIX_B;
+	struct small s = { .a = a, .b = b };
+
+	if (take_small_global(1, s, 2) != (__u64)a + b + 3)
+		return 1;
+
+	return 0;
+}
+
+__noinline __u64 take_upair_global(int a, union upair u, int c)
+{
+	return (__u64)a + u.parts.lo + u.parts.hi + c;
+}
+
+SEC("tc")
+__success __retval(0)
+int aggregate_arg_union_c_test(struct __sk_buff *skb)
+{
+	__u64 a = skb->len ^ MIX_A;
+	__u64 b = skb->len ^ MIX_B;
+	union upair u;
+
+	u.halves[0] = a;
+	u.halves[1] = b;
+	if (take_upair_global(1, u, 2) != a + b + 3)
+		return 1;
+
+	return 0;
+}
+
+#endif
+
 typedef unsigned __int128 u128;
 
 __noinline __u64 take_i128_global(int a, u128 v, int c)
@@ -28,4 +153,43 @@ int aggregate_arg_int128_c_test(struct __sk_buff *skb)
 	return 0;
 }
 
+#if defined(__BPF_FEATURE_STACK_ARGUMENT)
+
+static __noinline __u64 take_spilled_pair(int a, int b, int c, int d, struct pair p)
+{
+	return (__u64)a + b + c + d + p.lo + p.hi;
+}
+
+SEC("tc")
+__arch_x86_64 __arch_arm64
+__success __retval(0)
+int aggregate_arg_spilled_struct_c_test(struct __sk_buff *skb)
+{
+	__u64 a = skb->len ^ MIX_A;
+	__u64 b = skb->len ^ MIX_B;
+	struct pair p = { .lo = a, .hi = b };
+	int n = skb->len;
+
+	if (take_spilled_pair(n, n + 1, n + 2, n + 3, p) != a + b + 4 * n + 6)
+		return 1;
+
+	return 0;
+}
+
+#endif
+
+__noinline __u64 take_with_ptr_global(struct with_ptr s)
+{
+	return s.x;
+}
+
+SEC("tc")
+__failure __msg("type STRUCT in take_with_ptr_global() is not composed of scalars")
+int aggregate_arg_ptr_member_fail(struct __sk_buff *skb)
+{
+	struct with_ptr s = { .p = skb, .x = skb->len };
+
+	return take_with_ptr_global(s);
+}
+
 char _license[] SEC("license") = "GPL";
-- 
2.53.0-Meta

[PATCH bpf-next 11/12] selftests/bpf: Add inline-asm tests for by-value arguments

[Yonghong Song]

The tests cover a struct passed in a register pair, a pointer in one half
of it refused at the call site, a struct too large to pass by value, and
three placements a global function cannot have: a struct split between the
last argument register and the stack, one wholly past the registers, and
an __int128 whose two slots push the last parameter out.

GCC passes an aggregate by invisible reference, so a callee it compiles
expects a pointer where BTF says the halves of the struct are, and those
tests are left to clang.

Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
---
 .../selftests/bpf/prog_tests/aggregate_arg.c  |   9 +
 .../selftests/bpf/progs/aggregate_arg_func.c  | 155 ++++++++++++++++++
 2 files changed, 164 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/aggregate_arg.c
 create mode 100644 tools/testing/selftests/bpf/progs/aggregate_arg_func.c

diff --git a/tools/testing/selftests/bpf/prog_tests/aggregate_arg.c b/tools/testing/selftests/bpf/prog_tests/aggregate_arg.c
new file mode 100644
index 000000000000..b230f3bd3b2a
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/aggregate_arg.c
@@ -0,0 +1,9 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2026 Meta Platforms, Inc. and affiliates. */
+#include <test_progs.h>
+#include "aggregate_arg_func.skel.h"
+
+void test_aggregate_arg(void)
+{
+	RUN_TESTS(aggregate_arg_func);
+}
diff --git a/tools/testing/selftests/bpf/progs/aggregate_arg_func.c b/tools/testing/selftests/bpf/progs/aggregate_arg_func.c
new file mode 100644
index 000000000000..61dd5f86c3a4
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/aggregate_arg_func.c
@@ -0,0 +1,155 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2026 Meta Platforms, Inc. and affiliates. */
+#include <vmlinux.h>
+#include <bpf/bpf_helpers.h>
+#include "bpf_misc.h"
+
+typedef unsigned __int128 u128;
+
+struct pair {
+	__u64 lo;
+	__u64 hi;
+};
+
+struct too_big {
+	__u64 a;
+	__u64 b;
+	__u64 c;
+};
+
+#if defined(__clang__)
+
+__noinline __u64 global_arg_pair(int a, struct pair p, int c)
+{
+	return (__u64)a + p.lo + p.hi + c;
+}
+
+SEC("tc")
+__success __retval(0x33)
+__naked int aggregate_arg_pair_asm(void)
+{
+	asm volatile (
+	"r1 = 1;"
+	"r2 = 0x10;"	/* p.lo */
+	"r3 = 0x20;"	/* p.hi */
+	"r4 = 2;"
+	"call %[global_arg_pair];"
+	"exit;"
+	:
+	: __imm(global_arg_pair)
+	: __clobber_all);
+}
+
+SEC("tc")
+__failure __msg("R2 is not a scalar")
+__naked int aggregate_arg_pair_ptr_fail(void)
+{
+	asm volatile (
+	"r1 = 1;"
+	"r2 = r10;"	/* a stack pointer where p.lo belongs */
+	"r3 = 0x20;"
+	"r4 = 2;"
+	"call %[global_arg_pair];"
+	"exit;"
+	:
+	: __imm(global_arg_pair)
+	: __clobber_all);
+}
+
+#endif
+
+__noinline __u64 global_arg_too_big(struct too_big s)
+{
+	return s.a + s.b + s.c;
+}
+
+SEC("tc")
+__failure __msg("in global_arg_too_big() has size 24, only 1 to 16 bytes can be passed by value")
+__naked int aggregate_arg_too_big_fail(void)
+{
+	asm volatile (
+	"r1 = 0;"
+	"r2 = 0;"
+	"r3 = 0;"
+	"call %[global_arg_too_big];"
+	"r0 = 0;"
+	"exit;"
+	:
+	: __imm(global_arg_too_big)
+	: __clobber_all);
+}
+
+#if defined(__BPF_FEATURE_STACK_ARGUMENT)
+
+__noinline __u64 global_arg_split(int a, int b, int c, int d, struct pair p)
+{
+	return (__u64)a + b + c + d + p.lo + p.hi;
+}
+
+SEC("tc")
+__failure __msg("global function global_arg_split() needs 6 > 5 argument slots")
+__naked int aggregate_arg_split_fail(void)
+{
+	asm volatile (
+	"r1 = 0;"
+	"r2 = 0;"
+	"r3 = 0;"
+	"r4 = 0;"
+	"r5 = 0;"
+	"call %[global_arg_split];"
+	"r0 = 0;"
+	"exit;"
+	:
+	: __imm(global_arg_split)
+	: __clobber_all);
+}
+
+__noinline __u64 global_arg_past_regs(struct pair p, struct pair q, int a, struct pair r)
+{
+	return p.lo + p.hi + q.lo + q.hi + a + r.lo + r.hi;
+}
+
+SEC("tc")
+__failure __msg("global function global_arg_past_regs() needs 7 > 5 argument slots")
+__naked int aggregate_arg_past_regs_fail(void)
+{
+	asm volatile (
+	"r1 = 0;"
+	"r2 = 0;"
+	"r3 = 0;"
+	"r4 = 0;"
+	"r5 = 0;"
+	"call %[global_arg_past_regs];"
+	"r0 = 0;"
+	"exit;"
+	:
+	: __imm(global_arg_past_regs)
+	: __clobber_all);
+}
+
+__noinline __u64 global_arg_i128_slots(u128 v, int a, int b, int c, int d)
+{
+	return (__u64)v + a + b + c + d;
+}
+
+SEC("tc")
+__failure __msg("global function global_arg_i128_slots() needs 6 > 5 argument slots")
+__naked int aggregate_arg_i128_slots_fail(void)
+{
+	asm volatile (
+	"r1 = 0;"
+	"r2 = 0;"
+	"r3 = 0;"
+	"r4 = 0;"
+	"r5 = 0;"
+	"call %[global_arg_i128_slots];"
+	"r0 = 0;"
+	"exit;"
+	:
+	: __imm(global_arg_i128_slots)
+	: __clobber_all);
+}
+
+#endif
+
+char _license[] SEC("license") = "GPL";
-- 
2.53.0-Meta

[PATCH bpf-next 12/12] selftests/bpf: Add tests for by-value kfunc arguments

[Yonghong Song]

Add kfuncs taking a 16-byte struct and an __int128 by value, and tests
that call them, each checking the value the kfunc returns so that a
misplaced argument shows up rather than passing quietly.

They cover the placements the two calling conventions treat differently:
a struct in the middle of the argument list, an __int128 at an odd slot,
which AAPCS64 moves to an even register pair, a struct in the last two
argument registers, one the BPF convention splits between its last
register and the stack, which SysV moves wholly onto the stack, and the
same followed by another argument, which SysV then gives the register the
struct vacated. Two more cover the rejections that hold everywhere, an
aggregate holding a pointer and one too large to pass by value.

test_stack_arg_big() in stack_arg_fail.c passed a 16-byte struct as the
sixth argument and asserted the unrecognized stack argument type it used
to be reported as. It loads now, so the test is removed and the case is
covered by aggregate_arg_kfunc_straddle(), which makes the same call and
checks the value it returns.

Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
---
 .../selftests/bpf/prog_tests/aggregate_arg.c  |   2 +
 .../selftests/bpf/progs/aggregate_arg_kfunc.c | 106 ++++++++++++++++++
 .../testing/selftests/bpf/progs/arena_kfunc.c |  16 +++
 .../selftests/bpf/progs/stack_arg_fail.c      |  10 --
 .../selftests/bpf/test_kmods/bpf_testmod.c    |  40 +++++++
 .../bpf/test_kmods/bpf_testmod_kfunc.h        |  22 ++++
 6 files changed, 186 insertions(+), 10 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/progs/aggregate_arg_kfunc.c

diff --git a/tools/testing/selftests/bpf/prog_tests/aggregate_arg.c b/tools/testing/selftests/bpf/prog_tests/aggregate_arg.c
index b230f3bd3b2a..aa7562f48737 100644
--- a/tools/testing/selftests/bpf/prog_tests/aggregate_arg.c
+++ b/tools/testing/selftests/bpf/prog_tests/aggregate_arg.c
@@ -2,8 +2,10 @@
 /* Copyright (c) 2026 Meta Platforms, Inc. and affiliates. */
 #include <test_progs.h>
 #include "aggregate_arg_func.skel.h"
+#include "aggregate_arg_kfunc.skel.h"
 
 void test_aggregate_arg(void)
 {
 	RUN_TESTS(aggregate_arg_func);
+	RUN_TESTS(aggregate_arg_kfunc);
 }
diff --git a/tools/testing/selftests/bpf/progs/aggregate_arg_kfunc.c b/tools/testing/selftests/bpf/progs/aggregate_arg_kfunc.c
new file mode 100644
index 000000000000..6c2a07fad274
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/aggregate_arg_kfunc.c
@@ -0,0 +1,106 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2026 Meta Platforms, Inc. and affiliates. */
+#include <vmlinux.h>
+#include <bpf/bpf_helpers.h>
+#include "../test_kmods/bpf_testmod_kfunc.h"
+#include "bpf_misc.h"
+
+typedef unsigned __int128 u128;
+
+#define MIX_A	0xdeadbeefcafef00dULL
+#define MIX_B	0x0123456789abcdefULL
+
+#if defined(__clang__)
+
+SEC("tc")
+__arch_x86_64 __arch_arm64
+__success __retval(0)
+int aggregate_arg_kfunc_struct(struct __sk_buff *skb)
+{
+	__u64 a = skb->len ^ MIX_A;
+	__u64 b = skb->len ^ MIX_B;
+	struct prog_test_pair_arg s = { .lo = a, .hi = b };
+
+	if (bpf_kfunc_call_test_pair_arg(1, s, 2) != a + b + 3)
+		return 1;
+
+	return 0;
+}
+
+#endif
+
+SEC("tc")
+__arch_x86_64 __arch_arm64
+__success __retval(0)
+int aggregate_arg_kfunc_int128(struct __sk_buff *skb)
+{
+	__u64 a = skb->len ^ MIX_A;
+	__u64 b = skb->len ^ MIX_B;
+	u128 v = ((u128)a << 64) | b;
+
+	if (bpf_kfunc_call_test_i128_arg(1, 2, v) != a + b + 3)
+		return 1;
+
+	return 0;
+}
+
+#if defined(__clang__) && defined(__BPF_FEATURE_STACK_ARGUMENT)
+
+SEC("tc")
+__arch_x86_64 __arch_arm64
+__success __retval(0)
+int aggregate_arg_kfunc_last_regs(struct __sk_buff *skb)
+{
+	__u64 a = skb->len ^ MIX_A;
+	__u64 b = skb->len ^ MIX_B;
+	struct prog_test_pair_arg s = { .lo = a, .hi = b };
+
+	if (bpf_kfunc_call_test_pair_arg_nofit(1, 2, 3, 4, s) != a + b + 10)
+		return 1;
+
+	return 0;
+}
+
+SEC("tc")
+__arch_x86_64 __arch_arm64
+__success __retval(0)
+int aggregate_arg_kfunc_straddle(struct __sk_buff *skb)
+{
+	__u64 a = skb->len ^ MIX_A;
+	__u64 b = skb->len ^ MIX_B;
+	struct prog_test_big_arg s = { .a = a, .b = b };
+
+	if (bpf_kfunc_call_stack_arg_big(1, 2, 3, 4, 5, s) != a + b + 15)
+		return 1;
+
+	return 0;
+}
+
+SEC("tc")
+__arch_x86_64 __arch_arm64
+__success __retval(0)
+int aggregate_arg_kfunc_disorder(struct __sk_buff *skb)
+{
+	__u64 a = skb->len ^ MIX_A;
+	__u64 b = skb->len ^ MIX_B;
+	struct prog_test_pair_arg s = { .lo = a, .hi = b };
+
+	if (bpf_kfunc_call_test_pair_arg_disorder(1, 2, 3, 4, 5, s, 6) != a + b + 21)
+		return 1;
+
+	return 0;
+}
+
+#endif
+
+SEC("tc")
+__arch_x86_64 __arch_arm64
+__failure __msg("R1 type STRUCT is not composed of scalars")
+int aggregate_arg_kfunc_ptr_member(struct __sk_buff *skb)
+{
+	struct prog_test_ptr_arg s = { .p = skb, .x = 1 };
+
+	return bpf_kfunc_call_test_ptr_arg(s);
+}
+
+char _license[] SEC("license") = "GPL";
diff --git a/tools/testing/selftests/bpf/progs/arena_kfunc.c b/tools/testing/selftests/bpf/progs/arena_kfunc.c
index 50609f3b0564..8d1ce360a309 100644
--- a/tools/testing/selftests/bpf/progs/arena_kfunc.c
+++ b/tools/testing/selftests/bpf/progs/arena_kfunc.c
@@ -228,6 +228,22 @@ int arena_arg_stack(void *ctx)
 	bpf_kfunc_arena_stack_arg_test(1, 2, 3, 4, 5, (u64 *)1);
 	return 0;
 }
+
+#if defined(__clang__)
+/* The struct takes two slots, so the arena pointer is the sixth. */
+SEC("syscall")
+__arch_x86_64
+__arch_arm64
+__failure __msg("arena pointer cannot be a stack argument")
+int arena_arg_stack_after_pair(void *ctx)
+{
+	struct prog_test_pair_arg s = { .lo = 1, .hi = 2 };
+
+	bpf_arena_alloc_pages(&arena, NULL, 1, NUMA_NO_NODE, 0);
+	bpf_kfunc_call_test_pair_arena_arg(1, 2, 3, s, (u64 *)1);
+	return 0;
+}
+#endif
 #else
 SEC("syscall")
 __arch_x86_64
diff --git a/tools/testing/selftests/bpf/progs/stack_arg_fail.c b/tools/testing/selftests/bpf/progs/stack_arg_fail.c
index eed97d582515..fff2e947ea33 100644
--- a/tools/testing/selftests/bpf/progs/stack_arg_fail.c
+++ b/tools/testing/selftests/bpf/progs/stack_arg_fail.c
@@ -3,20 +3,10 @@
 
 #include <vmlinux.h>
 #include <bpf/bpf_helpers.h>
-#include "../test_kmods/bpf_testmod_kfunc.h"
 #include "bpf_misc.h"
 
 #if defined(__BPF_FEATURE_STACK_ARGUMENT)
 
-SEC("tc")
-__failure __msg("Unrecognized *(R11-8) type STRUCT")
-int test_stack_arg_big(struct __sk_buff *skb)
-{
-	struct prog_test_big_arg s = { .a = 1, .b = 2 };
-
-	return bpf_kfunc_call_stack_arg_big(1, 2, 3, 4, 5, s);
-}
-
 SEC("socket")
 __description("r11 in ALU instruction")
 __failure __msg("R11 is invalid")
diff --git a/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c b/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c
index 2380b6cbdead..390a9eba2e56 100644
--- a/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c
+++ b/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c
@@ -974,6 +974,40 @@ __bpf_kfunc struct prog_test_ret_pair bpf_kfunc_call_test_ret_fastcall(u64 a, u6
 	return r;
 }
 
+__bpf_kfunc u64 bpf_kfunc_call_test_pair_arg(u64 a, struct prog_test_pair_arg s, u64 b)
+{
+	return a + s.lo + s.hi + b;
+}
+
+__bpf_kfunc u64 bpf_kfunc_call_test_i128_arg(u64 a, u64 b, __int128 v)
+{
+	return a + b + (u64)((unsigned __int128)v >> 64) + (u64)v;
+}
+
+__bpf_kfunc u64 bpf_kfunc_call_test_pair_arg_nofit(u64 a, u64 b, u64 c, u64 d,
+						   struct prog_test_pair_arg s)
+{
+	return a + b + c + d + s.lo + s.hi;
+}
+
+__bpf_kfunc u64 bpf_kfunc_call_test_pair_arg_disorder(u64 a, u64 b, u64 c, u64 d, u64 e,
+						      struct prog_test_pair_arg s, u64 f)
+{
+	return a + b + c + d + e + s.lo + s.hi + f;
+}
+
+__bpf_kfunc u64 bpf_kfunc_call_test_ptr_arg(struct prog_test_ptr_arg s)
+{
+	return s.x;
+}
+
+__bpf_kfunc u64 bpf_kfunc_call_test_pair_arena_arg(u64 a, u64 b, u64 c,
+						   struct prog_test_pair_arg s,
+						   u64 *f__arena)
+{
+	return a + b + c + s.lo + s.hi + *f__arena;
+}
+
 __bpf_kfunc struct prog_test_ret_ptr bpf_kfunc_call_test_ret_ptr(u64 tag)
 {
 	struct prog_test_ret_ptr r = { .p = NULL, .tag = tag };
@@ -1612,6 +1646,12 @@ BTF_ID_FLAGS(func, bpf_kfunc_call_test_ret_arr_struct)
 BTF_ID_FLAGS(func, bpf_kfunc_call_test_ret_arr2d)
 BTF_ID_FLAGS(func, bpf_kfunc_call_test_ret_deep)
 BTF_ID_FLAGS(func, bpf_kfunc_call_test_ret_ii)
+BTF_ID_FLAGS(func, bpf_kfunc_call_test_pair_arg)
+BTF_ID_FLAGS(func, bpf_kfunc_call_test_i128_arg)
+BTF_ID_FLAGS(func, bpf_kfunc_call_test_pair_arg_nofit)
+BTF_ID_FLAGS(func, bpf_kfunc_call_test_pair_arg_disorder)
+BTF_ID_FLAGS(func, bpf_kfunc_call_test_ptr_arg)
+BTF_ID_FLAGS(func, bpf_kfunc_call_test_pair_arena_arg)
 #endif
 BTF_ID_FLAGS(func, bpf_kfunc_call_test_ret_big)
 BTF_ID_FLAGS(func, bpf_kfunc_call_stack_arg)
diff --git a/tools/testing/selftests/bpf/test_kmods/bpf_testmod_kfunc.h b/tools/testing/selftests/bpf/test_kmods/bpf_testmod_kfunc.h
index b213ef14848b..8ba684e5842f 100644
--- a/tools/testing/selftests/bpf/test_kmods/bpf_testmod_kfunc.h
+++ b/tools/testing/selftests/bpf/test_kmods/bpf_testmod_kfunc.h
@@ -61,6 +61,16 @@ struct prog_test_big_arg {
 	__u64 b;
 };
 
+struct prog_test_pair_arg {	/* 16 bytes: two argument registers */
+	__u64 lo;
+	__u64 hi;
+};
+
+struct prog_test_ptr_arg {	/* 16 bytes, but holds a pointer */
+	void *p;
+	__u64 x;
+};
+
 struct prog_test_ret_pair {	/* 16 bytes: R0:R2 */
 	__u64 lo;
 	__u64 hi;
@@ -221,6 +231,18 @@ __int128 bpf_kfunc_call_test_i128(__u64 a, __u64 b) __ksym;
 struct prog_test_ret_pair bpf_kfunc_call_test_ret_pair(__u64 a, __u64 b) __ksym;
 struct prog_test_ret_pair bpf_kfunc_call_test_ret_fastcall(__u64 a, __u64 b) __ksym;
 struct prog_test_ret_ii bpf_kfunc_call_test_ret_ii(int a, int b) __ksym;
+__u64 bpf_kfunc_call_test_pair_arg(__u64 a, struct prog_test_pair_arg s, __u64 b) __ksym;
+#ifdef __SIZEOF_INT128__
+__u64 bpf_kfunc_call_test_i128_arg(__u64 a, __u64 b, __int128 v) __ksym;
+#endif
+__u64 bpf_kfunc_call_test_pair_arg_nofit(__u64 a, __u64 b, __u64 c, __u64 d,
+					 struct prog_test_pair_arg s) __ksym;
+__u64 bpf_kfunc_call_test_pair_arg_disorder(__u64 a, __u64 b, __u64 c, __u64 d, __u64 e,
+					    struct prog_test_pair_arg s, __u64 f) __ksym;
+__u64 bpf_kfunc_call_test_ptr_arg(struct prog_test_ptr_arg s) __ksym;
+__u64 bpf_kfunc_call_test_pair_arena_arg(__u64 a, __u64 b, __u64 c,
+					 struct prog_test_pair_arg s,
+					 __u64 *f__arena) __ksym;
 struct prog_test_ret_ptr bpf_kfunc_call_test_ret_ptr(__u64 tag) __ksym;
 struct prog_test_ret_nested bpf_kfunc_call_test_ret_nested(__u64 tag) __ksym;
 struct prog_test_ret_ptr_arr bpf_kfunc_call_test_ret_ptr_arr(void) __ksym;
-- 
2.53.0-Meta

Re: [PATCH bpf-next 10/12] selftests/bpf: Add C tests for by-value arguments up to 16 bytes

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] `take_with_ptr_global` and its test are missing the `#if defined(__clang__)` guard used for other global functions taking structs by value, which will cause the test to fail when built with GCC.
- [Medium] `take_two_pairs_global` lacks the surrounding `int` arguments explicitly promised in the commit message to catch slot counting bugs.
--

commit a3823ad45ae612d86e218eeec89944cc894c7843
Author: Yonghong Song <yonghong.song@linux.dev>

selftests/bpf: Add C tests for by-value arguments up to 16 bytes

This commit extends the by-value argument tests with aggregate cases written
in C. It relies on the compiler to lower arguments into register pairs instead
of using hand-written register layouts.

> diff --git a/tools/testing/selftests/bpf/progs/verifier_int128_arg.c b/tools/testing/selftests/bpf/progs/verifier_int128_arg.c
> index 044fc4a800554..b10fb4aeb4056 100644
> --- a/tools/testing/selftests/bpf/progs/verifier_int128_arg.c
> +++ b/tools/testing/selftests/bpf/progs/verifier_int128_arg.c
[ ... ]
> +__noinline __u64 take_two_pairs_global(struct pair p, struct pair q)
> +{
> +	return p.lo + p.hi + q.lo + q.hi;
> +}
> +
> +SEC("tc")
> +__success __retval(0)
> +int aggregate_arg_two_structs_c_test(struct __sk_buff *skb)
> +{
> +	__u64 a = skb->len ^ MIX_A;
> +	__u64 b = skb->len ^ MIX_B;
> +	struct pair p = { .lo = a, .hi = b };
> +	struct pair q = { .lo = b, .hi = a };
> +
> +	if (take_two_pairs_global(p, q) != 2 * (a + b))
> +		return 1;
> +
> +	return 0;
> +}

[Severity: Medium]
Does this function signature and test implementation match the strategy
described in the commit message?

The commit message explicitly states a defensive testing strategy of having
"two struct arguments in a row, each with an int argument on either side so
that a wrong slot count shows up as a wrong value in the parameters around
it".

However, take_two_pairs_global completely omits these surrounding int
arguments. Can these be added to properly validate register slot counting?

[ ... ]
> +__noinline __u64 take_with_ptr_global(struct with_ptr s)
> +{
> +	return s.x;
> +}
> +
> +SEC("tc")
> +__failure __msg("type STRUCT in take_with_ptr_global() is not composed of scalars")
> +int aggregate_arg_ptr_member_fail(struct __sk_buff *skb)
> +{
> +	struct with_ptr s = { .p = skb, .x = skb->len };
> +
> +	return take_with_ptr_global(s);
> +}

[Severity: Medium]
Should take_with_ptr_global and its test aggregate_arg_ptr_member_fail
be enclosed within the #if defined(__clang__) guard?

Other global functions taking structs by value in this file are guarded.
Since GCC passes aggregates by invisible reference, it will produce a
different BTF signature (a pointer instead of a struct).

Will this cause the test to fail when compiled with GCC, as the verifier
might not emit the specific "type STRUCT ... is not composed of scalars"
failure message?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260904050957.3976119-1-yonghong.song@linux.dev?part=10

Re: [PATCH bpf-next 03/12] bpf: Support by-value struct arguments up to 16 bytes

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Out-of-bounds array write in `btf_prepare_func_args` when an argument passed by value requires multiple slots, overflowing the `sub->args` array.
--

commit d1c6556d5925344f81e73a6b534000285b7cee65
Author: Yonghong Song <yonghong.song@linux.dev>

bpf: Support by-value struct arguments up to 16 bytes

This commit adds support for passing struct arguments up to 16 bytes by value
to global BPF functions. The arguments are split into multiple register slots
based on their size.

> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> index 22828b489b778..dbd64cee1b27a 100644
> --- a/kernel/bpf/btf.c
> +++ b/kernel/bpf/btf.c
> @@ -8103,6 +8103,14 @@ int btf_prepare_func_args(struct bpf_verifier_env *env, int subprog)
>  	for (i = 0, slots_used = 0; i < nargs; i++) {
>  		u32 tags = 0;
>  
> +		if (slots_used >= MAX_BPF_FUNC_ARGS) {
> +			if (!is_global)
> +				return -EINVAL;
> +			bpf_log(log, "Arguments of %s() need more than %d argument slots\n",
> +				tname, MAX_BPF_FUNC_ARGS);
> +			return -EINVAL;
> +		}
> +

[ ... ]

> @@ -8231,6 +8239,31 @@ int btf_prepare_func_args(struct bpf_verifier_env *env, int subprog)
>  			sub->args[slots_used++].arg_type = ARG_ANYTHING;
>  			continue;
>  		}
> +		if (btf_type_is_struct(t)) {
> +			u32 nslots;
> +
> +			if (!t->size || t->size > 2 * BPF_REG_SIZE) {
> +				if (!is_global)
> +					return -EINVAL;
> +				bpf_log(log,
> +					"Arg#%d type %s in %s() has size %u, only 1 to %d bytes "
> +					"can be passed by value\n",
> +					i, btf_type_str(t), tname, t->size, 2 * BPF_REG_SIZE);
> +				return -EINVAL;
> +			}
> +			if (!btf_struct_is_composed_of(env, btf, t, BTF_MEMBER_SCALAR)) {
> +				if (!is_global)
> +					return -EINVAL;
> +				bpf_log(log, "Arg#%d type %s in %s() is not composed of scalars\n",
> +					i, btf_type_str(t), tname);
> +				return -EINVAL;
> +			}
> +
> +			nslots = (t->size + BPF_REG_SIZE - 1) / BPF_REG_SIZE;
> +			while (nslots--)
> +				sub->args[slots_used++].arg_type = ARG_ANYTHING;

[Severity: High]
Can this loop write out-of-bounds of the sub->args[] array?

The bounds check for slots_used is only performed at the beginning of the
outer for-loop in btf_prepare_func_args(). If a function argument is evaluated
when slots_used is exactly MAX_BPF_FUNC_ARGS - 1, and this argument is a
16-byte struct requiring 2 slots (nslots = 2), the inner while loop will
increment slots_used twice.

This will cause an out-of-bounds write when setting arg_type for the second
slot.

Since struct bpf_subprog_info defines args as having exactly
MAX_BPF_FUNC_ARGS elements, writing past the end will corrupt adjacent
memory fields like stack_arg_cnt.

Should the code verify if slots_used + nslots > MAX_BPF_FUNC_ARGS before
entering this loop?

> +			continue;
> +		}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260904050957.3976119-1-yonghong.song@linux.dev?part=3

Re: [PATCH bpf-next 06/12] bpf: Add a JIT helper for the outgoing stack of kfunc calls

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] An out-of-bounds write occurs in `btf_prepare_func_args` when processing a BPF subprogram with arguments that consume 11 slots followed by a final 16-byte struct argument.
--

commit d1c6556d5925344f81e73a6b534000285b7cee65
Author: Yonghong Song <yonghong.song@linux.dev>

bpf: Support by-value struct arguments up to 16 bytes

This commit adds support for global functions taking a struct or union by
value up to 16 bytes. It handles structs passed in consecutive registers
or split across registers and the stack, computing the necessary argument
slots. The stack argument count is then recomputed based on the slots
consumed to ensure proper JIT support checks.

> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> index 22828b489b778..dbd64cee1b27a 100644
> --- a/kernel/bpf/btf.c
> +++ b/kernel/bpf/btf.c
> @@ -8103,6 +8103,14 @@ int btf_prepare_func_args(struct bpf_verifier_env *env, int subprog)
>  	for (i = 0, slots_used = 0; i < nargs; i++) {
>  		u32 tags = 0;
>  
> +		if (slots_used >= MAX_BPF_FUNC_ARGS) {
> +			if (!is_global)
> +				return -EINVAL;
> +			bpf_log(log, "Arguments of %s() need more than %d argument slots\n",
> +				tname, MAX_BPF_FUNC_ARGS);
> +			return -EINVAL;
> +		}
> +
>  		err = btf_scan_decl_tags(env, btf, fn_t, i, is_global, &tags);
>  		if (err)
>  			return err;

[ ... ]

> @@ -8231,6 +8239,31 @@ int btf_prepare_func_args(struct bpf_verifier_env *env, int subprog)
>  			sub->args[slots_used++].arg_type = ARG_ANYTHING;
>  			continue;
>  		}
> +		if (btf_type_is_struct(t)) {
> +			u32 nslots;
> +
> +			if (!t->size || t->size > 2 * BPF_REG_SIZE) {
> +				if (!is_global)
> +					return -EINVAL;
> +				bpf_log(log,
> +					"Arg#%d type %s in %s() has size %u, only 1 to %d bytes "
> +					"can be passed by value\n",
> +					i, btf_type_str(t), tname, t->size, 2 * BPF_REG_SIZE);
> +				return -EINVAL;
> +			}
> +			if (!btf_struct_is_composed_of(env, btf, t, BTF_MEMBER_SCALAR)) {
> +				if (!is_global)
> +					return -EINVAL;
> +				bpf_log(log, "Arg#%d type %s in %s() is not composed of scalars\n",
> +					i, btf_type_str(t), tname);
> +				return -EINVAL;
> +			}
> +
> +			nslots = (t->size + BPF_REG_SIZE - 1) / BPF_REG_SIZE;
> +			while (nslots--)
> +				sub->args[slots_used++].arg_type = ARG_ANYTHING;

[Severity: High]
Does this loop write beyond the end of the sub->args[] array?

If a non-global BPF subprogram has arguments consuming exactly 11 slots,
slots_used >= MAX_BPF_FUNC_ARGS evaluates to false (11 >= 12) at the start
of the loop, allowing it to proceed.

When a final 16-byte struct argument follows, nslots is set to 2. This
while loop then writes to sub->args[11] and subsequently out-of-bounds to
sub->args[12].

Could this memory corruption overwrite the adjacent stack_arg_cnt and
max_out_stack_arg_cnt fields in struct bpf_subprog_info?

On big-endian architectures, this might artificially inflate
max_out_stack_arg_cnt to 9 (the value of ARG_ANYTHING), causing the
verifier to subsequently reject valid BPF programs in
check_outgoing_stack_args(). On little-endian architectures it would
silently zero the field.

> +			continue;
> +		}
>  		if (!is_global)
>  			return -EINVAL;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260904050957.3976119-1-yonghong.song@linux.dev?part=6

Re: [PATCH bpf-next 04/12] bpf: Support __int128 as a by-value function argument

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Out-of-bounds array write in `btf_prepare_func_args()` when an argument takes multiple register slots (like a 16-byte struct or `__int128`) and pushes `slots_used` past `MAX_BPF_FUNC_ARGS`.
--

commit 6ab0b55d70f74022fd34563ec7b71f1a649ea690
Author: Yonghong Song <yonghong.song@linux.dev>

bpf: Support __int128 as a by-value function argument

This commit adds support for 128-bit integers as by-value function arguments
in BPF. It handles __int128 similarly to a 16-byte struct, consuming two
consecutive argument registers.

> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> --- a/kernel/bpf/btf.c
> +++ b/kernel/bpf/btf.c
> @@ -8251,7 +8247,8 @@ int btf_prepare_func_args(struct bpf_verifier_env *env, int subprog)
>  					i, btf_type_str(t), tname, t->size, 2 * BPF_REG_SIZE);
>  				return -EINVAL;
>  			}
> -			if (!btf_struct_is_composed_of(env, btf, t, BTF_MEMBER_SCALAR)) {
> +			if (btf_type_is_struct(t) &&
> +			    !btf_struct_is_composed_of(env, btf, t, BTF_MEMBER_SCALAR)) {
>  				if (!is_global)
>  					return -EINVAL;
>  				bpf_log(log, "Arg#%d type %s in %s() is not composed of scalars\n",

[Severity: High]
This is a pre-existing issue, but this patch enables __int128 arguments which
trigger the same vulnerable path as 16-byte structs.

Can this loop overflow the sub->args array when an argument takes multiple
slots?

Looking at btf_prepare_func_args() just below this hunk:

			nslots = (t->size + BPF_REG_SIZE - 1) / BPF_REG_SIZE;
			while (nslots--)
				sub->args[slots_used++].arg_type = ARG_ANYTHING;

If a BPF program defines a subprogram with exactly 12 arguments, where the
last argument is a 16-byte struct or __int128, slots_used would be 11 when
processing the final argument.

The 16-byte argument yields nslots = 2. The loop then increments slots_used
twice without checking bounds mid-loop. Because sub->args is sized
MAX_BPF_FUNC_ARGS (12), this writes to sub->args[11] and then out of bounds
to sub->args[12].

Since sub->args sits at the end of struct bpf_subprog_info, immediately
followed by stack_arg_cnt, this out-of-bounds write corrupts stack_arg_cnt.

Later, sub->stack_arg_cnt is set to slots_used - MAX_BPF_FUNC_REG_ARGS,
which rewrites the corrupted enum value. Depending on endianness, this can
result in an invalid enum value triggering a verifier_bug(), or an incorrect
type expectation (like ARG_PTR_TO_CTX instead of a scalar).

Could we add a bounds check inside or before the nslots loop to prevent
slots_used from exceeding MAX_BPF_FUNC_ARGS?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260904050957.3976119-1-yonghong.song@linux.dev?part=4

Re: [PATCH bpf-next 07/12] bpf, x86: Place kfunc arguments per the SysV calling convention

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] emit_kfunc_args() and bpf_jit_kfunc_stack_slots() silently ignore kfunc_arg_layout() errors for kfuncs requiring more than 12 argument eightbytes, leading to miscompiled BPF programs with incorrect argument placements.
--

commit f5201014b232baff8f1ba36c0b74d2c3e1e5ccf9
Author: Yonghong Song <yonghong.song@linux.dev>

bpf, x86: Place kfunc arguments per the SysV calling convention

This commit updates the BPF JIT on x86 to conform to the SysV calling
convention when placing kfunc arguments. It adjusts argument placements
before a kfunc call to account for differing rules between BPF and SysV
regarding stack and register usage. The outgoing argument area is sized
to support both conventions.

> diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
> --- a/arch/x86/net/bpf_jit_comp.c
> +++ b/arch/x86/net/bpf_jit_comp.c
[ ... ]
> +static int emit_kfunc_args(const struct btf_func_model *fm, u8 **pprog,
> +			   s32 stack_base)
> +{
> +	u8 *prog = *pprog, *start = prog;
> +	bool done[MAX_BPF_FUNC_ARGS] = {};
> +	u8 pos[MAX_BPF_FUNC_ARGS];
> +	int i, j, n, todo = 0, parked = -1;
> +
> +	n = kfunc_arg_layout(fm, pos, ARRAY_SIZE(pos));
> +	if (n < 0)
> +		return 0;

[Severity: Medium]
Will this silently ignore errors if a kfunc requires more than 12 argument
eightbytes?

While the verifier limits the argument count to 12 (MAX_BPF_FUNC_ARGS),
arguments can take up to 2 slots, so the total slots could reach 24.
If a BPF program calls a registered kfunc that requires more than 12
eightbyte slots (such as 7 struct arguments of 16 bytes each),
kfunc_arg_layout() will return -EINVAL because max is hardcoded to 12 here.

Since emit_kfunc_args() returns 0 instead of propagating the error code,
could this cause the BPF program to compile successfully but execute with
missing argument shuffle instructions? It seems the kfunc might read garbage
from the 6th slot onwards, potentially causing panics or data corruption.

This same pattern of silently ignoring layout errors appears in
bpf_jit_kfunc_stack_slots():

kernel/bpf/core.c:bpf_jit_kfunc_stack_slots() {
	...
		n = layout(fm, pos, ARRAY_SIZE(pos));
		if (n < 0)
			continue;
	...
}

Does this also leave stack calculations undersized?

Though the stack corruption is mathematically averted because BPF's calling
convention stack requirements always equal or exceed SysV's (making
outgoing_rsp naturally sufficient), the silent argument corruption remains.
As there are currently no mainline kfuncs that take more than 12 slots,
this appears to be a latent regression.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260904050957.3976119-1-yonghong.song@linux.dev?part=7

Re: [PATCH bpf-next 08/12] bpf: Record a 16-byte argument alignment in the function model

[bot+bpf-ci]

[-- Attachment #1: Type: text/plain, Size: 2858 bytes --]

> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index 3a7eb2185c35..5b93aa97f1bc 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -1193,6 +1193,9 @@ struct bpf_prog_offload {
>  	u32			jited_len;
>  };
>
> +/* Argument aligned to 16 bytes. */
> +#define BTF_FMODEL_ALIGN16_ARG		BIT(0)
> +
>  /* The argument is signed. */
>  #define BTF_FMODEL_SIGNED_ARG		BIT(1)

[ ... ]

> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> index 6c391449e298..f933a675b0a8 100644
> --- a/kernel/bpf/btf.c
> +++ b/kernel/bpf/btf.c
> @@ -7633,6 +7633,15 @@ int btf_distill_func_proto(struct bpf_verifier_log *log,
>  		}
>  		m->arg_size[i] = ret;
>  		m->arg_flags[i] = __get_arg_fmodel_flags(btf, &args[i], t);
> +
> +		align16 = btf_type_align16(btf, t, 0);
> +		if (align16 < 0) {
> +			bpf_log(log, "The function %s arg%d type %s has unknown alignment.\n",
> +				tname, i, btf_type_str(t));
> +			return -EINVAL;
> +		} else if (align16) {
> +			m->arg_flags[i] |= BTF_FMODEL_ALIGN16_ARG;
> +		}
>  	}
>  	m->nr_args = nargs;
>  	return 0;

[ ... ]

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 030fb37c22ef..461883158ffa 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -11748,6 +11748,31 @@ bool btf_struct_is_composed_of(struct bpf_verifier_env *env,
>  	return btf_struct_member_walk(env, btf, t, member_kinds, 0, NULL);
>  }
>
> +int btf_type_align16(const struct btf *btf, const struct btf_type *t, int rec)
> +{
> +	const struct btf_member *member;
> +	int ret;
> +	u32 i;
> +
> +	while (btf_type_is_array(t))
> +		t = btf_type_skip_modifiers(btf, btf_array(t)->type, NULL);
> +
> +	if (btf_type_is_int(t))
> +		return t->size > 8;
> +	if (!btf_type_is_struct(t))
> +		return 0;
> +	if (rec >= BTF_MEMBER_MAX_DEPTH)
> +		return -E2BIG;
> +
> +	for_each_member(i, t, member) {
> +		ret = btf_type_align16(btf, btf_type_skip_modifiers(btf, member->type, NULL),
> +				       rec + 1);
> +		if (ret)
> +			return ret;
> +	}
> +	return 0;
> +}

This isn't a bug, but btf_type_align16() looks structurally similar to
kfunc_arg_align() further down in the same file. Both walk types with the
same pattern: strip arrays with `while (btf_type_is_array(t))`, test for
`btf_type_is_int(t)` with `t->size > 8`, bail on `!btf_type_is_struct(t)`,
check `rec >= BTF_MEMBER_MAX_DEPTH`, and recurse through members via
`btf_type_skip_modifiers(btf, member->type, NULL)`. Could the two share one
walk, and would hiding the `rec` argument behind a wrapper keep the exported
signature simpler, the way btf_struct_is_composed_of() wraps
btf_struct_member_walk()?


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/33840714551

Re: [PATCH bpf-next 11/12] selftests/bpf: Add inline-asm tests for by-value arguments

[bot+bpf-ci]

[-- Attachment #1: Type: text/plain, Size: 3732 bytes --]

> diff --git a/tools/testing/selftests/bpf/prog_tests/aggregate_arg.c b/tools/testing/selftests/bpf/prog_tests/aggregate_arg.c
> new file mode 100644
> index 000000000000..b230f3bd3b2a
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/aggregate_arg.c
> @@ -0,0 +1,9 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (c) 2026 Meta Platforms, Inc. and affiliates. */
> +#include <test_progs.h>
> +#include "aggregate_arg_func.skel.h"
> +
> +void test_aggregate_arg(void)
> +{
> +	RUN_TESTS(aggregate_arg_func);
> +}
> diff --git a/tools/testing/selftests/bpf/progs/aggregate_arg_func.c b/tools/testing/selftests/bpf/progs/aggregate_arg_func.c
> new file mode 100644
> index 000000000000..61dd5f86c3a4
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/aggregate_arg_func.c
> @@ -0,0 +1,155 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (c) 2026 Meta Platforms, Inc. and affiliates. */
> +#include <vmlinux.h>
> +#include <bpf/bpf_helpers.h>
> +#include "bpf_misc.h"
> +
> +typedef unsigned __int128 u128;
> +
> +struct pair {
> +	__u64 lo;
> +	__u64 hi;
> +};

[ ... ]

> +#if defined(__BPF_FEATURE_STACK_ARGUMENT)
> +
> +__noinline __u64 global_arg_split(int a, int b, int c, int d, struct pair p)
> +{
> +	return (__u64)a + b + c + d + p.lo + p.hi;
> +}
> +
> +SEC("tc")
> +__failure __msg("global function global_arg_split() needs 6 > 5 argument slots")
> +__naked int aggregate_arg_split_fail(void)

[ ... ]

> +SEC("tc")
> +__failure __msg("global function global_arg_past_regs() needs 7 > 5 argument slots")
> +__naked int aggregate_arg_past_regs_fail(void)

[ ... ]

> +SEC("tc")
> +__failure __msg("global function global_arg_i128_slots() needs 6 > 5 argument slots")
> +__naked int aggregate_arg_i128_slots_fail(void)

[ ... ]

A subsystem pattern flags this as potentially concerning: this commit
creates a new BPF prog file (progs/aggregate_arg_func.c) plus a new
test_progs entry point (prog_tests/aggregate_arg.c) for by-value
argument tests, while the immediately preceding commit in the same series
(7bdde6e0b83d) added by-value aggregate argument tests to the existing
progs/verifier_int128_arg.c, which is already run by
prog_tests/verifier.c.

This means coverage for one feature is now split across two test runners
in the same directory, and struct pair is defined twice
(progs/verifier_int128_arg.c and progs/aggregate_arg_func.c).

Three of the six new subtests (aggregate_arg_split_fail,
aggregate_arg_past_regs_fail, aggregate_arg_i128_slots_fail) are pure
__failure/__msg verifier tests that need no hand-written register layout,
so they could fit as cases in the existing file. The same is true of
aggregate_arg_pair_ptr_fail.

Is there a reason to create a separate test file rather than adding these
tests to the existing progs/verifier_int128_arg.c, which already covers
by-value aggregate arguments and would consolidate the feature's tests in
one place?

(For context: the pattern is motivated by the fact that aggregate_arg_pair_asm
and aggregate_arg_pair_ptr_fail need hand-written register placement that
C cannot express, and this layout mirrors the existing aggregate_ret
structure with prog_tests/aggregate_ret.c + progs/aggregate_ret_func.c.
A following commit extends the test with kfunc cases, so the new entry
point is reused rather than single-purpose. The question is whether the
C tests in verifier_int128_arg.c should move here too, so the feature's
tests live in one place and struct pair is defined once.)


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/33840714551

Re: [PATCH bpf-next 04/12] bpf: Support __int128 as a by-value function argument

[bot+bpf-ci]

[-- Attachment #1: Type: text/plain, Size: 1204 bytes --]

> bpf: Support __int128 as a by-value function argument
>
> A 128-bit integer follows the same calling convention as a 16-byte
> by-value struct: LLVM emits it as a 16-byte BTF_KIND_INT and passes it in
> two consecutive argument registers. So a __int128 gets its two slots.
>
> The test added at the start of the series, which recorded the wrong
> register map as an R4 !read_ok rejection, now passes and is flipped to
> __success.
>
> Signed-off-by: Yonghong Song <yonghong.song@linux.dev>

This looks like a bug fix for the multi-slot struct argument support added
in an earlier commit. Should this include:

  Fixes: c9bf7bb8555b ("bpf: Support by-value struct arguments up to 16 bytes")

The earlier commit added multi-slot support for struct arguments but
incorrectly treated all BTF_KIND_INT types (including __int128) as
single-slot arguments. A 16-byte __int128 should consume two argument
slots like a 16-byte struct, which is what this patch corrects.


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/33840714551

Re: [PATCH bpf-next 10/12] selftests/bpf: Add C tests for by-value arguments up to 16 bytes

[bot+bpf-ci]

[-- Attachment #1: Type: text/plain, Size: 3757 bytes --]

> diff --git a/tools/testing/selftests/bpf/progs/verifier_int128_arg.c b/tools/testing/selftests/bpf/progs/verifier_int128_arg.c
> index 044fc4a80055..b10fb4aeb405 100644
> --- a/tools/testing/selftests/bpf/progs/verifier_int128_arg.c
> +++ b/tools/testing/selftests/bpf/progs/verifier_int128_arg.c

[ ... ]

> @@ -28,4 +153,43 @@ int aggregate_arg_int128_c_test(struct __sk_buff *skb)
>  	return 0;
>  }
>
> +#if defined(__BPF_FEATURE_STACK_ARGUMENT)
> +
> +static __noinline __u64 take_spilled_pair(int a, int b, int c, int d, struct pair p)
> +{
> +	return (__u64)a + b + c + d + p.lo + p.hi;
> +}
> +
> +SEC("tc")
> +__arch_x86_64 __arch_arm64
> +__success __retval(0)
> +int aggregate_arg_spilled_struct_c_test(struct __sk_buff *skb)

Should aggregate_arg_spilled_struct_c_test() include the __load_if_JITed()
tag?

The test calls take_spilled_pair(int a, int b, int c, int d, struct pair p)
which has 5 BTF parameters but 6 argument slots. btf_prepare_func_args() in
kernel/bpf/btf.c expands the 16-byte struct into two slots:

    nslots = (t->size + BPF_REG_SIZE - 1) / BPF_REG_SIZE;
    while (nslots--)
        sub->args[slots_used++].arg_type = ARG_ANYTHING;
    ...
    if (slots_used > MAX_BPF_FUNC_REG_ARGS) {
        ...
        sub->stack_arg_cnt = slots_used - MAX_BPF_FUNC_REG_ARGS;

This path is reached for a static callee too: check_func_call() calls
btf_check_subprog_call() -> btf_check_func_arg_match() ->
btf_prepare_func_args() before the bpf_subprog_is_global() branch in
kernel/bpf/verifier.c, and then propagates the count into
caller_info->stack_arg_cnt. So bpf_in_stack_arg_cnt() returns 1 for
take_spilled_pair().

bpf_fixup_call_args() then rejects the program outright when the JIT is
not used:

    for (i = 0; i < env->subprog_cnt; i++) {
        if (bpf_in_stack_arg_cnt(&env->subprog_info[i])) {
            verbose(env, "stack args are not supported in non-JITed programs\n");
            return -EINVAL;
        }
    }

This is in kernel/bpf/fixups.c, inside #ifndef CONFIG_BPF_JIT_ALWAYS_ON,
reached whenever prog->jit_requested is 0.

On the harness side, test_loader.c derives the runtime from the sysctl
and only skips programs that asked for the JIT:

    int current_runtime = is_jit_enabled() ? JITED : NO_JITED;
    ...
    if ((current_runtime & spec->load_mask) == 0) { test__skip(); return; }

and spec->load_mask defaults to JITED | NO_JITED. __arch_x86_64 and
__arch_arm64 only constrain spec->arch_mask, not load_mask.

So with /proc/sys/net/core/bpf_jit_enable == 0 and CONFIG_BPF_JIT_ALWAYS_ON=n,
the program is loaded, rejected with "stack args are not supported in
non-JITed programs", and the __success expectation fails.

The idiom for this is __load_if_JITed() (bpf_misc.h). The sibling file
progs/verifier_aggregate_ret.c tags every one of its __success __retval
cases with __load_if_JITed() for exactly this reason. Adding
__load_if_JITed() alongside __arch_x86_64 __arch_arm64 here would turn
the JIT-off run into a skip.

The same tag also appears to be missing on the stack-argument cases added
later in the series (progs/aggregate_arg_kfunc.c:
aggregate_arg_kfunc_last_regs, aggregate_arg_kfunc_straddle), and on the
pre-existing progs/verifier_stack_arg.c __success cases.

> +{
> +	__u64 a = skb->len ^ MIX_A;
> +	__u64 b = skb->len ^ MIX_B;
> +	struct pair p = { .lo = a, .hi = b };
> +	int n = skb->len;
> +
> +	if (take_spilled_pair(n, n + 1, n + 2, n + 3, p) != a + b + 4 * n + 6)
> +		return 1;
> +
> +	return 0;
> +}
> +
> +#endif

[ ... ]


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/33840714551

Re: [PATCH bpf-next 03/12] bpf: Support by-value struct arguments up to 16 bytes

[bot+bpf-ci]

[-- Attachment #1: Type: text/plain, Size: 1619 bytes --]

> A global function taking a struct or union by value is rejected today:
>
>   Arg#1 type STRUCT in tar() is not supported yet.
>
> A struct of at most 16 bytes arrives in one or two consecutive
> registers. Currently, the struct is composed of scalars.
>
> The slots of a value are independent of each other, so the compiler may
> split one across the last argument register and the stack, as in
>
>   static void f(int a, int b, int c, int d, struct pair p);
>
> or place it wholly past the registers, and the verifier describes either
> the same way. What has to follow the slots is stack_arg_cnt, recomputed
> from the slots consumed, together with the "no stack args in global
> functions" and JIT support checks.

This isn't a bug, but could the wording be more precise? The phrase
"Currently, the struct is composed of scalars" reads as a statement of
fact about a particular struct, but the code enforces it as a requirement
on every by-value argument through btf_struct_is_composed_of() in
btf_prepare_func_args(). Would it help to say that only scalar-composed
structs are accepted for now?

Also, the changelog doesn't mention that sub->arg_cnt changes meaning from
the BTF parameter count to the argument-slot count, which is the reason
the kernel/bpf/verifier.c hunk is needed at all. Would noting this change
help reviewers understand the verifier.c modification?


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/33840714551

Re: [PATCH bpf-next 09/12] bpf, arm64: Place kfunc arguments per AAPCS64

[bot+bpf-ci]

[-- Attachment #1: Type: text/plain, Size: 3274 bytes --]

> diff --git a/arch/arm64/net/bpf_jit_comp.c b/arch/arm64/net/bpf_jit_comp.c
> index 3aa3ea0bc30b..f6c783d176ed 100644
> --- a/arch/arm64/net/bpf_jit_comp.c
> +++ b/arch/arm64/net/bpf_jit_comp.c

[ ... ]

> @@ -1256,6 +1256,93 @@ static void emit_stack_arg_store_imm(s32 imm, s16 bpf_off, const u8 tmp, struct
>  	}
>  }
>
> +/* The kernel ABI hands the first eight eightbytes of arguments to registers. */
> +static const u8 a64_arg_reg[8] = {
> +	A64_R(0), A64_R(1), A64_R(2), A64_R(3),
> +	A64_R(4), A64_R(5), A64_R(6), A64_R(7),
> +};
> +
> +static int kfunc_arg_layout(const struct btf_func_model *fm, u8 *pos, int max)
> +{
> +	int i, k, ngrn = 0, nsaa = 0, slot = 0;
> +
> +	for (i = 0; i < fm->nr_args; i++) {
> +		bool align16 = fm->arg_flags[i] & BTF_FMODEL_ALIGN16_ARG;
> +		int n = (fm->arg_size[i] + 7) / 8;
> +
> +		if (slot + n > max)
> +			return -EINVAL;
> +		if (align16)
> +			ngrn = round_up(ngrn, 2);
> +		if (ngrn + n <= 8) {
> +			for (k = 0; k < n; k++)
> +				pos[slot++] = ngrn++;
> +			continue;
> +		}
> +		/* Nothing that follows gets a register either. */
> +		ngrn = 8;
> +		if (align16)
> +			nsaa = round_up(nsaa, 2);
> +		for (k = 0; k < n; k++)
> +			pos[slot++] = 8 + nsaa++;
> +	}
> +	return slot;
> +}
> +
> +static u16 kfunc_arg_stack_bytes(const struct bpf_prog *prog)
> +{
> +	u16 slots = bpf_jit_kfunc_stack_slots(prog, 8, kfunc_arg_layout);
> +
> +	return round_up(slots * sizeof(u64), 16);
> +}
> +
> +static void emit_arg_pos_load(u8 reg, u8 pos, struct jit_ctx *ctx)
> +{
> +	if (pos < 8)
> +		emit(A64_MOV(1, reg, a64_arg_reg[pos]), ctx);
> +	else
> +		emit(A64_LDR64I(reg, A64_SP, (pos - 8) * sizeof(u64)), ctx);
> +}
> +
> +static void emit_arg_pos_store(u8 pos, u8 reg, struct jit_ctx *ctx)
> +{
> +	if (pos < 8)
> +		emit(A64_MOV(1, a64_arg_reg[pos], reg), ctx);
> +	else
> +		emit(A64_STR64I(reg, A64_SP, (pos - 8) * sizeof(u64)), ctx);
> +}

This isn't a bug, but is the pos < 8 branch in emit_arg_pos_store()
kept deliberately for symmetry with emit_arg_pos_load() and with the
x86 helper, or could it be dropped?

Looking at emit_kfunc_args() below, emit_arg_pos_store() is only ever
called from the pos[i] >= 8 branch:

> +static int emit_kfunc_args(const struct bpf_insn *insn, struct jit_ctx *ctx)
> +{
> +	const u8 tmp = bpf2a64[TMP_REG_1];
> +	const struct btf_func_model *fm;
> +	u8 pos[MAX_BPF_FUNC_ARGS];
> +	int i, n;
> +
> +	fm = bpf_jit_find_kfunc_model(ctx->prog, insn);
> +	if (!fm)
> +		return -EINVAL;
> +
> +	n = kfunc_arg_layout(fm, pos, ARRAY_SIZE(pos));
> +	if (n < 0)
> +		return 0;
> +
> +	for (i = n - 1; i >= 0; i--) {
> +		if (pos[i] == i)
> +			continue;
> +		if (WARN_ON_ONCE(pos[i] < i))
> +			return -EFAULT;
> +		if (pos[i] < 8) {
> +			/* into a register, read straight from the slot */
> +			emit_arg_pos_load(a64_arg_reg[pos[i]], i, ctx);
> +		} else {
> +			emit_arg_pos_load(tmp, i, ctx);
> +			emit_arg_pos_store(pos[i], tmp, ctx);
> +		}
> +	}
> +	return 0;
> +}

so pos is always at least 8 and the register store is never emitted.

[ ... ]


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/33840714551

Re: [PATCH bpf-next 05/12] bpf: Support by-value struct and __int128 kfunc arguments

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Incorrect buffer register resolution for memory bounds check in check_kfunc_args() due to confusion between BTF argument index and register slot index.
- [High] bpf_kfunc_stack_access_bytes() incorrectly uses register slot indices to index BTF arguments, breaking stack liveness tracking.
- [High] bpf_get_call_summary() incorrectly uses the number of BTF arguments instead of the number of argument slots for kfuncs, breaking register liveness tracking.
--

commit 225a338e077323b335756911c91748fab0c8ec13
Author: Yonghong Song <yonghong.song@linux.dev>

bpf: Support by-value struct and __int128 kfunc arguments

A kfunc taking a struct or union by value is rejected today, and one
taking an __int128 is accepted but mis-counted:

  Unrecognized R2 type STRUCT

Give the kfunc argument walk the same slot count a BPF-to-BPF call now
uses: one argument register per eightbyte, so a 16-byte value takes two.
The outgoing stack argument count at the call site follows the slots for
the same reason. Similar to BPF-to-BPF aggregate handling, a kfunc
aggregate argument only supports tbe composed of scalars.

Signed-off-by: Yonghong Song <yonghong.song@linux.dev>

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 17e576c7b7572..030fb37c22ef1 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
[ ... ]
> @@ -12792,29 +12904,34 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_call_arg_me
[ ... ]
>  	/* Check that BTF function arguments match actual types that the
>  	 * verifier sees.
>  	 */
> -	for (i = 0; i < nargs; i++) {
> -		struct bpf_reg_state *reg = get_func_arg_reg(caller, regs, i);
> +	for (i = 0, slots_used = 0; i < nargs; i++, slots_used += nslots) {
> +		struct bpf_reg_state *reg = get_func_arg_reg(caller, regs, slots_used);
>  		const struct btf_type *t, *ref_t, *resolve_ret;
>  		enum bpf_arg_type arg_type = ARG_DONTCARE;
> -		argno_t argno = argno_from_arg(i + 1);
> +		argno_t argno = argno_from_arg(slots_used + 1);

[Severity: High]
This change separates the BTF argument index (i) from the register slot
index (slots_used), but does it leave other parts of the loop using i
incorrectly?

For instance, further down in this loop, case KF_ARG_MEM_SIZE uses i - 1
instead of slots_used - 1 to find the buffer pointer:

    case KF_ARG_MEM_SIZE:
    {
        struct bpf_reg_state *buff_reg = get_func_arg_reg(caller, regs, i - 1);
        struct bpf_reg_state *size_reg = reg;

Could this cause a kfunc memory bounds check to validate the size argument
against the memory bounds of the wrong pointer register if a preceding
argument consumed multiple slots?

[ ... ]
> @@ -14308,11 +14444,11 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
>  	if (bpf_is_kfunc_pkt_changing(&meta))
>  		clear_all_pkt_pointers(env);
>  
> -	nargs = btf_type_vlen(meta.func_proto);
> -	if (nargs > MAX_BPF_FUNC_REG_ARGS) {
> +	proto_slots = kfunc_proto_slots(desc_btf, meta.func_proto);
> +	if (proto_slots > MAX_BPF_FUNC_REG_ARGS) {

[Severity: High]
This patch introduces multi-slot kfunc arguments, but were all places
that map register slots to BTF arguments updated?

For example, in bpf_kfunc_stack_access_bytes(), the register slot index
(arg) is still directly used to index the BTF argument array:

	args = btf_params(meta.func_proto);
	nargs = btf_type_vlen(meta.func_proto);
	if (arg >= nargs)
		return 0;

	t = btf_type_skip_modifiers(btf, args[arg].type, NULL);

Could this result in looking up the wrong parameter or going out of bounds
when querying stack accesses for kfuncs with multi-slot arguments?

[Severity: High]
And similarly, does bpf_get_call_summary() also need to be updated?

bool bpf_get_call_summary(...) {
...
	if (bpf_pseudo_kfunc_call(call)) {
...
		cs->num_params = btf_type_vlen(meta.func_proto);

Since liveness tracking uses cs->num_params to determine the number of live
registers, could using the BTF argument count instead of the number of
register slots cause the verifier to treat upper registers of multi-slot
arguments as dead?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260904050957.3976119-1-yonghong.song@linux.dev?part=5

Re: [PATCH bpf-next 05/12] bpf: Support by-value struct and __int128 kfunc arguments

[bot+bpf-ci]

[-- Attachment #1: Type: text/plain, Size: 6188 bytes --]

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 17e576c7b757..030fb37c22ef 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c

[ ... ]

> @@ -12792,29 +12904,34 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_call_arg_me
>  	const struct btf *btf = meta->btf;
>  	const struct btf_param *args;
>  	struct btf_record *rec;
> -	u32 i, nargs;
> +	u32 i, k, nargs, proto_slots, slots_used, nslots = 0;
>  	int ret;
>
>  	args = (const struct btf_param *)(meta->func_proto + 1);
>  	nargs = btf_type_vlen(meta->func_proto);
> +	proto_slots = kfunc_proto_slots(btf, meta->func_proto);
>
> -	ret = check_outgoing_stack_args(env, caller, nargs, func_name, btf, args);
> +	ret = check_outgoing_stack_args(env, caller, proto_slots, func_name, btf,
> +					proto_slots == nargs ? args : NULL);
>  	if (ret)
>  		return ret;
>
>  	/* Check that BTF function arguments match actual types that the
>  	 * verifier sees.
>  	 */
> -	for (i = 0; i < nargs; i++) {
> -		struct bpf_reg_state *reg = get_func_arg_reg(caller, regs, i);
> +	for (i = 0, slots_used = 0; i < nargs; i++, slots_used += nslots) {
> +		struct bpf_reg_state *reg = get_func_arg_reg(caller, regs, slots_used);
>  		const struct btf_type *t, *ref_t, *resolve_ret;
>  		enum bpf_arg_type arg_type = ARG_DONTCARE;
> -		argno_t argno = argno_from_arg(i + 1);
> +		argno_t argno = argno_from_arg(slots_used + 1);
>  		int regno = reg_from_argno(argno);
>  		bool btf_id_fixed_off_ok = true;
>  		u32 ref_id = args[i].type, type_size;
>  		int kf_arg_type = meta->fn->arg_type[i];
>
> +		t = btf_type_skip_modifiers(btf, args[i].type, NULL);
> +		nslots = kfunc_arg_slots(t);
> +

This converts the argument walk to slot-based indexing, where 'reg' and
'argno' now come from 'slots_used' rather than 'i'.  Before this patch
every parameter took exactly one slot, so parameter index and slot
index were the same number.  After this patch they diverge whenever a
parameter takes two slots.

The KF_ARG_MEM_SIZE case, ~500 lines further down in check_kfunc_args(),
was not converted.  It still resolves the buffer register by parameter
index:

kernel/bpf/verifier.c:check_kfunc_args() {
    ...
    case KF_ARG_MEM_SIZE:
    {
        struct bpf_reg_state *buff_reg = get_func_arg_reg(caller, regs, i - 1);
        struct bpf_reg_state *size_reg = reg;
        argno_t buff_argno = argno_from_arg(i);
    ...
}

For a kfunc such as

    void bpf_x(__int128 v, void *p, u32 p__sz);

v occupies slots 0-1 (R1, R2), p slot 2 (R3), p__sz slot 3 (R4).  At
i == 2, slots_used == 3, so 'reg' and 'argno' correctly select R4, but
buff_reg = get_func_arg_reg(caller, regs, i - 1) selects R2 (the upper
eightbyte of v) instead of R3, the pointer the length actually
describes.  The same applies to a 16-byte by-value struct parameter,
exactly the shape this patch exists to enable.

The consequence is a validation hole, not just a bad diagnostic.  R2 is
a SCALAR_VALUE, so when the program passes a __int128 (or struct) whose
upper eightbyte is zero -- the common case for any value that fits in
64 bits -- bpf_register_is_null(buff_reg) is true and the case breaks
out early:

        if (bpf_register_is_null(buff_reg))
            break;

check_mem_size_reg() is then never called, so neither p nor p__sz is
checked.  The pointer's own iteration does not cover it either:
get_kfunc_arg_type() classifies a pointer followed by a __sz parameter
as plain KF_ARG_PTR_TO_MEM without MEM_FIXED_SIZE, and that switch arm
only validates size when MEM_FIXED_SIZE is set.  The program therefore
loads and the kfunc runs with an unbounded length against p.  When the
upper eightbyte is non-zero the failure inverts into a spurious
rejection, since check_mem_size_reg() is handed a scalar as the memory
pointer.  buff_argno = argno_from_arg(i) is wrong in the same way and
feeds the wrong register into the diagnostics and into
mark_stack_arg_precision()/mark_chain_precision() via
check_mem_size_reg().

At this commit gen_kfunc_arg_proto() gates every nslots > 1 argument on
bpf_jit_supports_kfunc_arg_slot(), whose __weak default in
kernel/bpf/core.c returns false, so multi-slot kfunc arguments are
still refused.  Within the stated range
db649eef61418e5d0289d88bcdd663e510756589..9ad88f2069ccb8b9d464230a3d04082baaffe4e0,
commit 2924cde3ca70 ("bpf, x86: Place kfunc arguments per the SysV
calling convention") and commit e958bc6f838b ("bpf, arm64: Place kfunc
arguments per AAPCS64") enable the path, and no commit in that range
touches kernel/bpf/verifier.c again apart from db9461ba111c adding
btf_type_align16().

Both uses should be derived from the slot walk, e.g.
get_func_arg_reg(caller, regs, slots_used - 1) and
argno_from_arg(slots_used).

---

commit db649eef61418e5d0289d88bcdd663e510756589
Author: Yonghong Song <yonghong.song@linux.dev>

>     bpf: Support by-value struct and __int128 kfunc arguments
>
>     A kfunc taking a struct or union by value is rejected today, and one
>     taking an __int128 is accepted but mis-counted:
>
>       Unrecognized R2 type STRUCT
>
>     Give the kfunc argument walk the same slot count a BPF-to-BPF call now
>     uses: one argument register per eightbyte, so a 16-byte value takes two.
>     The outgoing stack argument count at the call site follows the slots for
>     the same reason. Similar to BPF-to-BPF aggregate handling, a kfunc
>     aggregate argument only supports tbe composed of scalars.
>
>     Signed-off-by: Yonghong Song <yonghong.song@linux.dev>

This looks like a bug fix for kfunc argument handling introduced in
commit 1690dcf27c73 ("bpf: Classify scalar kfunc arguments from BTF"),
which added get_kfunc_arg_type() that only handled scalar and pointer
types, causing struct/union types to be rejected with 'Unrecognized R2
type STRUCT' and __int128 arguments to be mis-counted.  Should this
include:

  Fixes: 1690dcf27c73 ("bpf: Classify scalar kfunc arguments from BTF")


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/33840714551