[PATCH bpf-next 0/5] bpf: Follow up by-value struct and __int128 argument support

[PATCH bpf-next 0/5] bpf: Follow up by-value struct and __int128 argument support

[Yonghong Song]

This is a follow-up to the series "bpf: Support by-value struct and
__int128 arguments" [1], which placed kfunc arguments by the kernel
calling convention in the x86-64 and arm64 JITs, and made the arm64
trampoline place an argument aligned to 16 bytes: an __int128, or an
aggregate holding one. This series covers what was left:

  - The x86-64 trampoline. x86-64 starts such an argument passed on the
    stack at a 16-byte aligned slot, which can leave an empty slot
    before it, but the trampoline copied the stack arguments as if
    there were none. A bpf program attached to such a function read the
    wrong values from the empty slot on, and with fexit, where the
    trampoline calls the traced function itself, that function got its
    stack arguments one slot off as well.

  - A trampoline on a BPF function. BPF code calls one with the
    arguments in the BPF convention, one slot after another, but the
    arm64 trampoline placed them by the kernel convention, and the
    x86-64 fix above would have done the same.

  - Tests with an __int128 at an odd slot, where arm64 leaves a register
    or a stack slot empty. On arm64 such a kfunc gets BTF only with the
    pahole patch [2], now in pahole master after v1.32, so these tests
    were not part of [1].

Patch 1 adds the odd-slot kfunc tests. Patch 2 keeps a BPF function's
trampoline arguments in the BPF convention, and patch 3 places the
x86-64 trampoline arguments by the calling convention. Patches 4 and 5
test trampolines on kernel functions and on BPF functions with an
argument after a 16-byte aligned one.

Tested on x86-64 with test_progs.

  [1] https://lore.kernel.org/bpf/20260912195156.980886-1-yonghong.song@linux.dev/
  [2] https://lore.kernel.org/dwarves/20260917164043.1577508-1-yonghong.song@linux.dev/

Yonghong Song (5):
  selftests/bpf: Test by-value __int128 kfunc arguments at an odd slot
  bpf: Keep the BPF calling convention for a trampoline on a BPF
    function
  bpf, x86: Place trampoline arguments by the x86-64 calling convention
  selftests/bpf: Test trampoline args around a 16-byte aligned stack arg
  selftests/bpf: Test trampoline args of a BPF function at an odd slot

 arch/x86/net/bpf_jit_comp.c                   | 189 +++++++++---------
 include/linux/bpf.h                           |   2 +
 kernel/bpf/btf.c                              |   1 +
 kernel/bpf/core.c                             |  12 ++
 kernel/bpf/verifier.c                         |   1 +
 .../selftests/bpf/prog_tests/tracing_struct.c |  96 ++++++++-
 .../selftests/bpf/progs/aggregate_arg_kfunc.c |  71 +++++++
 .../bpf/progs/tracing_struct_int128.c         |  45 +++++
 .../bpf/progs/tracing_struct_int128_tgt.c     |  68 +++++++
 .../bpf/progs/tracing_struct_int128_tracer.c  |  44 ++++
 .../selftests/bpf/test_kmods/bpf_testmod.c    |  69 +++++++
 .../bpf/test_kmods/bpf_testmod_kfunc.h        |  13 ++
 12 files changed, 515 insertions(+), 96 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/progs/tracing_struct_int128_tgt.c
 create mode 100644 tools/testing/selftests/bpf/progs/tracing_struct_int128_tracer.c

-- 
2.53.0-Meta

[PATCH bpf-next 1/5] selftests/bpf: Test by-value __int128 kfunc arguments at an odd slot

[Yonghong Song]

This follows up the series "bpf: Support by-value struct and __int128
arguments" [1]. The __int128 kfunc tests it added pass the __int128 at
an even argument slot, after two u64s, or past the argument registers,
where it lands on an even arm64 stack slot. So neither the AAPCS64 rule
that rounds the register number up to an even one for an argument
aligned to 16 bytes, nor the same rule for a stack slot, is exercised.
Each leaves a hole the arm64 JIT has to open by moving the __int128 and
every argument after it up a slot.

Add three kfuncs. Like the earlier ones, each weighs its arguments by
argument slot, so that two eightbytes landing in each other's slots are
caught as well as one landing in a hole:

  - bpf_kfunc_call_test_i128_arg_odd(): the __int128 at an odd register
    slot.
  - bpf_kfunc_call_test_i128_arg_odd_many(): the same, with arguments
    after it that arm64 shifts onto its stack, and a second __int128 at
    an odd arm64 stack slot.
  - bpf_kfunc_call_test_i128_arg_stack_hole(): a struct holding an
    __int128 at an odd stack slot on both x86-64 and arm64, after x86-64
    gives R9 back to an argument following a 16-byte struct.

x86-64 has no register alignment rule and places the first two like the
BPF convention, so they check that its JIT leaves them where they are.
The last two need more than the five BPF argument registers and are only
built with a compiler that supports stack arguments.

On arm64 the first two kfuncs get BTF only with the pahole patch
"dwarf_loader: Skip the argument register the arm64 ABI leaves as a hole"
[2], in pahole master after v1.32. An older pahole finds the argument
after the __int128 in an unexpected register, x4 rather than x3, and
leaves the function out of BTF, so the tests fail to load there.

Link: https://lore.kernel.org/bpf/20260912195156.980886-1-yonghong.song@linux.dev/ [1]
Link: https://lore.kernel.org/dwarves/20260917164043.1577508-1-yonghong.song@linux.dev/ [2]
Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
---
 .../selftests/bpf/progs/aggregate_arg_kfunc.c | 71 +++++++++++++++++++
 .../selftests/bpf/test_kmods/bpf_testmod.c    | 26 +++++++
 .../bpf/test_kmods/bpf_testmod_kfunc.h        | 13 ++++
 3 files changed, 110 insertions(+)

diff --git a/tools/testing/selftests/bpf/progs/aggregate_arg_kfunc.c b/tools/testing/selftests/bpf/progs/aggregate_arg_kfunc.c
index 52d3d5d53946..c87e886423f4 100644
--- a/tools/testing/selftests/bpf/progs/aggregate_arg_kfunc.c
+++ b/tools/testing/selftests/bpf/progs/aggregate_arg_kfunc.c
@@ -50,6 +50,27 @@ int aggregate_arg_kfunc_int128(struct __sk_buff *skb)
 	return 0;
 }
 
+/*
+ * arm64 rounds the register number up to an even one for an argument
+ * aligned to 16 bytes, so it wants this __int128 in x2 and x3, leaving x1
+ * empty, and the last argument in x4. The x86-64 ABI has no such rule.
+ */
+SEC("tc")
+__arch_x86_64 __arch_arm64
+__load_if_JITed()
+__success __retval(0)
+int aggregate_arg_kfunc_int128_odd(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_odd(1, v, 2) != 1 + 2 * b + 3 * a + 4 * 2)
+		return 1;
+
+	return 0;
+}
+
 #endif /* __SIZEOF_INT128__ */
 
 #if defined(__clang__) && defined(__BPF_FEATURE_STACK_ARGUMENT)
@@ -131,6 +152,56 @@ int aggregate_arg_kfunc_split8(struct __sk_buff *skb)
 
 #ifdef __SIZEOF_INT128__
 
+/*
+ * The hole of aggregate_arg_kfunc_int128_odd, with arguments after it that
+ * arm64 shifts through x5 to x7, where its JIT keeps the BPF stack slots,
+ * and onto its stack. The second __int128 then leaves the second arm64
+ * stack slot empty. x86-64 packs the eightbytes like the BPF convention,
+ * so nothing moves there.
+ */
+SEC("tc")
+__arch_x86_64 __arch_arm64
+__load_if_JITed()
+__success __retval(0)
+int aggregate_arg_kfunc_int128_odd_many(struct __sk_buff *skb)
+{
+	__u64 a = skb->len ^ MIX_A;
+	__u64 b = skb->len ^ MIX_B;
+	u128 v = ((u128)a << 64) | b;
+	u128 w = ((u128)~a << 64) | ~b;
+
+	if (bpf_kfunc_call_test_i128_arg_odd_many(1, v, 2, 3, 4, 5, 6, w) !=
+	    1 + 2 * b + 3 * a + 4 * 2 + 5 * 3 + 6 * 4 + 7 * 5 + 8 * 6 +
+	    9 * ~b + 10 * ~a)
+		return 1;
+
+	return 0;
+}
+
+/*
+ * x86-64 moves s to the stack and gives R9 back to the seventh argument.
+ * w, aligned to 16 bytes by the __int128 it holds, then leaves a stack slot
+ * empty before it: the fourth on x86-64, the second on arm64.
+ */
+SEC("tc")
+__arch_x86_64 __arch_arm64
+__load_if_JITed()
+__success __retval(0)
+int aggregate_arg_kfunc_int128_stack_hole(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 };
+	struct prog_test_i128_arg w = { .v = ((u128)~a << 64) | ~b };
+
+	if (bpf_kfunc_call_test_i128_arg_stack_hole(1, 2, 3, 4, 5, s, 6, 7, w) !=
+	    1 + 2 * 2 + 3 * 3 + 4 * 4 + 5 * 5 + 6 * a + 7 * b + 8 * 6 + 9 * 7 +
+	    10 * ~b + 11 * ~a)
+		return 1;
+
+	return 0;
+}
+
 /*
  * Both conventions pad the stack to align this __int128, and the BPF
  * convention pads for neither, so both JITs move it up an eightbyte.
diff --git a/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c b/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c
index 93847ca6293b..54a5fc39e0e2 100644
--- a/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c
+++ b/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c
@@ -1040,6 +1040,29 @@ __bpf_kfunc u64 bpf_kfunc_call_test_i128_arg(u64 a, u64 b, __int128 v)
 	return a + b * 2 + (u64)v * 3 + (u64)((unsigned __int128)v >> 64) * 4;
 }
 
+__bpf_kfunc u64 bpf_kfunc_call_test_i128_arg_odd(u64 a, __int128 v, u64 b)
+{
+	return a + (u64)v * 2 + (u64)((unsigned __int128)v >> 64) * 3 + b * 4;
+}
+
+__bpf_kfunc u64 bpf_kfunc_call_test_i128_arg_odd_many(u64 a, __int128 v, u64 b, u64 c,
+						      u64 d, u64 e, u64 f, __int128 w)
+{
+	return a + (u64)v * 2 + (u64)((unsigned __int128)v >> 64) * 3 +
+	       b * 4 + c * 5 + d * 6 + e * 7 + f * 8 +
+	       (u64)w * 9 + (u64)((unsigned __int128)w >> 64) * 10;
+}
+
+__bpf_kfunc u64 bpf_kfunc_call_test_i128_arg_stack_hole(u64 a, u64 b, u64 c, u64 d, u64 e,
+							struct prog_test_pair_arg s,
+							u64 f, u64 g,
+							struct prog_test_i128_arg w)
+{
+	return a + b * 2 + c * 3 + d * 4 + e * 5 + s.lo * 6 + s.hi * 7 +
+	       f * 8 + g * 9 + (u64)w.v * 10 +
+	       (u64)((unsigned __int128)w.v >> 64) * 11;
+}
+
 __bpf_kfunc u64 bpf_kfunc_call_test_i128_arg_pad(u64 a, u64 b, u64 c, u64 d, u64 e,
 						 u64 f, u64 g, __int128 v)
 {
@@ -1763,6 +1786,9 @@ 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_i128_arg_odd)
+BTF_ID_FLAGS(func, bpf_kfunc_call_test_i128_arg_odd_many)
+BTF_ID_FLAGS(func, bpf_kfunc_call_test_i128_arg_stack_hole)
 BTF_ID_FLAGS(func, bpf_kfunc_call_test_i128_arg_pad)
 BTF_ID_FLAGS(func, bpf_kfunc_call_test_pair_arg_nofit)
 BTF_ID_FLAGS(func, bpf_kfunc_call_test_pair_arg_tail)
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 67c02a421d13..a6a93c901416 100644
--- a/tools/testing/selftests/bpf/test_kmods/bpf_testmod_kfunc.h
+++ b/tools/testing/selftests/bpf/test_kmods/bpf_testmod_kfunc.h
@@ -66,6 +66,12 @@ struct prog_test_pair_arg {	/* 16 bytes: two argument registers */
 	__u64 hi;
 };
 
+#ifdef __SIZEOF_INT128__
+struct prog_test_i128_arg {	/* 16 bytes, aligned to 16 like its member */
+	__int128 v;
+};
+#endif
+
 struct prog_test_ptr_arg {	/* 16 bytes, but holds a pointer */
 	void *p;
 	__u64 x;
@@ -234,6 +240,13 @@ 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;
+__u64 bpf_kfunc_call_test_i128_arg_odd(__u64 a, __int128 v, __u64 b) __ksym;
+__u64 bpf_kfunc_call_test_i128_arg_odd_many(__u64 a, __int128 v, __u64 b, __u64 c,
+					    __u64 d, __u64 e, __u64 f, __int128 w) __ksym;
+__u64 bpf_kfunc_call_test_i128_arg_stack_hole(__u64 a, __u64 b, __u64 c, __u64 d,
+					      __u64 e, struct prog_test_pair_arg s,
+					      __u64 f, __u64 g,
+					      struct prog_test_i128_arg w) __ksym;
 __u64 bpf_kfunc_call_test_i128_arg_pad(__u64 a, __u64 b, __u64 c, __u64 d, __u64 e,
 				       __u64 f, __u64 g, __int128 v) __ksym;
 #endif
-- 
2.53.0-Meta

[PATCH bpf-next 2/5] bpf: Keep the BPF calling convention for a trampoline on a BPF function

[Yonghong Song]

A trampoline on a BPF function, fentry or fexit attached through
attach_prog_fd, takes the function's arguments from where BPF code left
them. A call from one BPF function to another passes them in the BPF
convention, one argument slot after another: only a kfunc call is moved
into the kernel convention. bpf_jit_place_args(), which the arm64
trampoline uses since commit afdbb4cdd3d2 ("bpf, arm64: Place trampoline
arguments by the arm64 calling convention"), places them by the kernel
convention instead. For a BPF function taking a 16-byte aligned argument
at an odd slot, such as

	u64 f(u64 a, struct { __int128 v; } s, u64 b)

the trampoline reads s from x2 and x3 where the caller left it in x1 and
x2, and b from x4, so the program sees the wrong values, and an fexit
trampoline hands the function shifted arguments.

Mark the model of a target program's function as bpf_tgt_prog, and have
bpf_jit_place_args() leave every slot of such a model where the BPF
convention put it. That is where both JITs pass it in a call from one
BPF function to another: slot N where the kernel convention passes its
Nth eightbyte, in a register or a stack slot. The x86-64 trampoline does
not use bpf_jit_place_args() yet; the next patch makes it, and this
keeps it right for a BPF target.

Fixes: afdbb4cdd3d2 ("bpf, arm64: Place trampoline arguments by the arm64 calling convention")
Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
---
 include/linux/bpf.h   |  2 ++
 kernel/bpf/btf.c      |  1 +
 kernel/bpf/core.c     | 12 ++++++++++++
 kernel/bpf/verifier.c |  1 +
 4 files changed, 16 insertions(+)

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 54144372281c..6dbc09356e60 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -1260,6 +1260,8 @@ struct btf_func_model {
 	u8 ret_size;
 	u8 ret_flags;
 	u8 nr_args;
+	/* a function of a BPF target program, called in the BPF convention */
+	bool bpf_tgt_prog;
 	u8 arg_size[MAX_BPF_FUNC_ARGS];
 	u8 arg_flags[MAX_BPF_FUNC_ARGS];
 };
diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index 0630675377aa..3a05cf2fde8e 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -8019,6 +8019,7 @@ int btf_distill_func_proto(struct bpf_verifier_log *log,
 	u32 i, nargs;
 	int ret;
 
+	m->bpf_tgt_prog = false;
 	if (!func) {
 		/* BTF function prototype doesn't match the verifier types.
 		 * Fall back to MAX_BPF_FUNC_REG_ARGS u64 args.
diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
index 05c89396119a..22dced3f59f3 100644
--- a/kernel/bpf/core.c
+++ b/kernel/bpf/core.c
@@ -3342,6 +3342,18 @@ u32 bpf_jit_place_args(const struct bpf_jit_arg_abi *abi,
 	u32 i, k, nslots, slot = 0, nregs_used = 0, stack_off = 0;
 	bool on_stack = false;
 
+	/*
+	 * A BPF function is called by BPF code, which passes slot N where
+	 * the kernel convention passes its Nth eightbyte, whatever the
+	 * arguments: no hole, no split refused and no register backfilled.
+	 */
+	if (fm->bpf_tgt_prog) {
+		for (i = 0; i < fm->nr_args; i++)
+			for (k = btf_func_model_arg_slots(fm, i); k > 0; k--, slot++)
+				pos_of_slot[slot] = slot;
+		return slot;
+	}
+
 	for (i = 0; i < fm->nr_args; i++) {
 		bool align16 = fm->arg_flags[i] & BTF_FMODEL_ALIGN16_ARG;
 		u32 pos;
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 353bde9ae227..50a06c2d3f7f 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -21620,6 +21620,7 @@ int bpf_check_attach_target(struct bpf_verifier_log *log,
 		ret = btf_distill_func_proto(log, btf, t, tname, &tgt_info->fmodel);
 		if (ret < 0)
 			return ret;
+		tgt_info->fmodel.bpf_tgt_prog = !!tgt_prog;
 
 		if (tgt_info->fmodel.ret_size > 8 &&
 		    attach_uses_trampoline_retval(prog->expected_attach_type)) {
-- 
2.53.0-Meta

[PATCH bpf-next 3/5] bpf, x86: Place trampoline arguments by the x86-64 calling convention

[Yonghong Song]

save_args() numbers the on-stack argument slots consecutively, and the
area built for the original function is sized the same way, while the
x86-64 ABI rounds the stack slot up to an even one for an argument
aligned to 16 bytes: an __int128, or an aggregate holding one. When such
an argument lands on an odd stack slot, it and every argument after it
are read from the wrong place, so a bpf program attached to the function
reads a neighbouring eightbyte, and the arguments handed on to the
original function are shifted. For example:

	/* s holds an __int128 */
	void foo(u64 a, u64 b, u64 c, u64 d, u64 e, u64 f, u64 g,
		 struct s h, u64 i)

x86-64 puts g in the first stack slot, leaves the second empty and puts
h in the third and fourth, while the trampoline reads h from the second
and third.

Take the position of each argument slot from bpf_jit_place_args(), which
knows the x86-64 rules, backfill included. save_args() then reads each
slot from its register or incoming stack slot, restore_regs() puts back
only the register slots, and the area built for the original function
mirrors the incoming one, holes included.

Since the previous patch, bpf_jit_place_args() keeps a BPF function's
arguments in the BPF convention, which also puts right a 16-byte
argument that BPF code splits across r9 and the stack.

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

diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
index 083fcd6cf15b..0fc7715e569b 100644
--- a/arch/x86/net/bpf_jit_comp.c
+++ b/arch/x86/net/bpf_jit_comp.c
@@ -1906,6 +1906,16 @@ static const u8 x86_arg_reg[] = {
 	BPF_REG_1, BPF_REG_2, BPF_REG_3, BPF_REG_4, BPF_REG_5, X86_REG_R9,
 };
 
+static bool x86_arg_on_stack(u8 pos)
+{
+	return pos >= x86_arg_abi.nr_arg_regs;
+}
+
+static s32 x86_arg_stack_off(u8 pos)
+{
+	return (pos - x86_arg_abi.nr_arg_regs) * 8;
+}
+
 /*
  * Move the arguments the x86-64 ABI places somewhere other than the argument
  * slot the BPF calling convention gave them. @stack_base addresses the
@@ -3393,21 +3403,17 @@ static void clean_stack_garbage(const struct btf_func_model *m,
 	}
 }
 
-/* get the count of the regs that are used to pass arguments */
-static int get_nr_used_regs(const struct btf_func_model *m)
+/* the eightbytes taken by the on-stack arguments, alignment holes included */
+static int get_nr_stack_slots(const u8 *pos_of_slot, int nr_slots)
 {
-	int i, arg_regs, nr_used_regs = 0;
-
-	for (i = 0; i < min_t(int, m->nr_args, MAX_BPF_FUNC_ARGS); i++) {
-		arg_regs = (m->arg_size[i] + 7) / 8;
-		if (nr_used_regs + arg_regs <= 6)
-			nr_used_regs += arg_regs;
+	int slot;
 
-		if (nr_used_regs >= 6)
-			break;
+	for (slot = nr_slots - 1; slot >= 0; slot--) {
+		if (x86_arg_on_stack(pos_of_slot[slot]))
+			return x86_arg_stack_off(pos_of_slot[slot]) / 8 + 1;
 	}
 
-	return nr_used_regs;
+	return 0;
 }
 
 /*
@@ -3437,20 +3443,21 @@ static void emit_arena_arg_conv(u8 **pprog, u32 src_reg, bool nullable, u32 base
 }
 
 static void save_args(const struct btf_func_model *m, u8 **prog,
-		      int stack_size, bool for_call_origin, u32 flags,
-		      u64 arena_base)
+		      const u8 *pos_of_slot, int stack_size,
+		      bool for_call_origin, u32 flags, u64 arena_base)
 {
-	int arg_regs, first_off = 0, nr_regs = 0, nr_stack_slots = 0;
+	int arg_regs, first_off = 0, nr_stack_slots = 0;
 	bool use_jmp = bpf_trampoline_use_jmp(flags);
 	int stack_args_off = (use_jmp || (flags & BPF_TRAMP_F_INDIRECT)) ? 16 : 24;
-	int i, j;
+	int bargs_off = stack_size;
+	int i, j, slot, off, doff;
 
 	/* Store function arguments to stack.
 	 * For a function that accepts two pointers the sequence will be:
 	 * mov QWORD PTR [rbp-0x10],rdi
 	 * mov QWORD PTR [rbp-0x8],rsi
 	 */
-	for (i = 0; i < min_t(int, m->nr_args, MAX_BPF_FUNC_ARGS); i++) {
+	for (i = 0, slot = 0; i < m->nr_args; i++) {
 		bool arena_arg = arena_base && (m->arg_flags[i] & BTF_FMODEL_ARENA_ARG);
 		bool nullable = m->arg_flags[i] & BTF_FMODEL_NULLABLE_ARG;
 
@@ -3472,8 +3479,35 @@ static void save_args(const struct btf_func_model *m, u8 **prog,
 		 *
 		 * the arg1-5,arg7 will be passed by regs, and arg6 will
 		 * by stack.
+		 *
+		 * An argument aligned to 16 bytes, an __int128 or a struct
+		 * holding one, also starts at an even stack slot, which can
+		 * leave a hole before it. bpf_jit_place_args() worked out
+		 * where each eightbyte is.
 		 */
-		if (nr_regs + arg_regs > 6) {
+		for (j = 0; j < arg_regs; j++, slot++, bargs_off -= 8) {
+			u8 pos = pos_of_slot[slot];
+			u32 src;
+
+			if (!x86_arg_on_stack(pos)) {
+				/* Only copy the arguments on-stack to
+				 * current 'stack_size' and ignore the regs,
+				 * used to prepare the arguments on-stack for
+				 * origin call.
+				 */
+				if (for_call_origin)
+					continue;
+
+				/* copy the arguments from regs into stack */
+				src = x86_arg_reg[pos];
+				if (arena_arg) {
+					emit_arena_arg_conv(prog, src, nullable, (u32)arena_base);
+					src = BPF_REG_0;
+				}
+				emit_stx(prog, BPF_DW, BPF_REG_FP, src, -bargs_off);
+				continue;
+			}
+
 			/* copy function arguments from origin stack frame
 			 * into current stack frame.
 			 *
@@ -3483,53 +3517,33 @@ static void save_args(const struct btf_func_model *m, u8 **prog,
 			 * entered through the fentry call, so rbp + 24, and
 			 * a single one when it is entered with a jmp or
 			 * called indirectly, so rbp + 16.
+			 *
+			 * The area for the origin call mirrors the incoming
+			 * one, hole and all; only the bpf program takes the
+			 * arguments packed.
 			 */
-			for (j = 0; j < arg_regs; j++) {
-				emit_ldx(prog, BPF_DW, BPF_REG_0, BPF_REG_FP,
-					 nr_stack_slots * 8 + stack_args_off);
-				if (arena_arg)
-					emit_arena_arg_conv(prog, BPF_REG_0, nullable,
-							    (u32)arena_base);
-				emit_stx(prog, BPF_DW, BPF_REG_FP, BPF_REG_0,
-					 -stack_size);
-
-				if (!nr_stack_slots)
-					first_off = stack_size;
-				stack_size -= 8;
-				nr_stack_slots++;
-			}
-		} else {
-			/* Only copy the arguments on-stack to current
-			 * 'stack_size' and ignore the regs, used to
-			 * prepare the arguments on-stack for origin call.
-			 */
-			if (for_call_origin) {
-				nr_regs += arg_regs;
-				continue;
-			}
-
-			/* copy the arguments from regs into stack */
-			for (j = 0; j < arg_regs; j++) {
-				u32 src = nr_regs == 5 ? X86_REG_R9 : BPF_REG_1 + nr_regs;
-
-				if (arena_arg) {
-					emit_arena_arg_conv(prog, src, nullable, (u32)arena_base);
-					src = BPF_REG_0;
-				}
-				emit_stx(prog, BPF_DW, BPF_REG_FP, src, -stack_size);
-				stack_size -= 8;
-				nr_regs++;
-			}
+			off = x86_arg_stack_off(pos);
+			doff = for_call_origin ? stack_size - off : bargs_off;
+			emit_ldx(prog, BPF_DW, BPF_REG_0, BPF_REG_FP,
+				 stack_args_off + off);
+			if (arena_arg)
+				emit_arena_arg_conv(prog, BPF_REG_0, nullable,
+						    (u32)arena_base);
+			emit_stx(prog, BPF_DW, BPF_REG_FP, BPF_REG_0, -doff);
+
+			if (!nr_stack_slots)
+				first_off = doff;
+			nr_stack_slots++;
 		}
 	}
 
 	clean_stack_garbage(m, prog, nr_stack_slots, first_off);
 }
 
-static void restore_regs(const struct btf_func_model *m, u8 **prog,
+static void restore_regs(u8 **prog, const u8 *pos_of_slot, int nr_slots,
 			 int stack_size)
 {
-	int i, j, arg_regs, nr_regs = 0;
+	int slot;
 
 	/* Restore function arguments from stack.
 	 * For a function that accepts two pointers the sequence will be:
@@ -3538,23 +3552,10 @@ static void restore_regs(const struct btf_func_model *m, u8 **prog,
 	 *
 	 * The logic here is similar to what we do in save_args()
 	 */
-	for (i = 0; i < min_t(int, m->nr_args, MAX_BPF_FUNC_ARGS); i++) {
-		arg_regs = (m->arg_size[i] + 7) / 8;
-		if (nr_regs + arg_regs <= 6) {
-			for (j = 0; j < arg_regs; j++) {
-				emit_ldx(prog, BPF_DW,
-					 nr_regs == 5 ? X86_REG_R9 : BPF_REG_1 + nr_regs,
-					 BPF_REG_FP,
-					 -stack_size);
-				stack_size -= 8;
-				nr_regs++;
-			}
-		} else {
-			stack_size -= 8 * arg_regs;
-		}
-
-		if (nr_regs >= 6)
-			break;
+	for (slot = 0; slot < nr_slots; slot++, stack_size -= 8) {
+		if (!x86_arg_on_stack(pos_of_slot[slot]))
+			emit_ldx(prog, BPF_DW, x86_arg_reg[pos_of_slot[slot]],
+				 BPF_REG_FP, -stack_size);
 	}
 }
 
@@ -3813,7 +3814,8 @@ static int __arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *rw_im
 					 struct bpf_tramp_nodes *tnodes,
 					 void *func_addr)
 {
-	int i, ret, nr_regs = m->nr_args, stack_size = 0;
+	int i, ret, nr_regs, stack_size = 0;
+	u8 pos_of_slot[MAX_BPF_FUNC_ARG_SLOTS];
 	int regs_off, func_meta_off, ip_off, run_ctx_off, arg_stack_off, rbx_off;
 	struct bpf_tramp_nodes *fentry = &tnodes[BPF_TRAMP_FENTRY];
 	struct bpf_tramp_nodes *fexit = &tnodes[BPF_TRAMP_FEXIT];
@@ -3836,12 +3838,13 @@ static int __arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *rw_im
 
 	arena_base = bpf_tramp_arena_base(m, tnodes, flags);
 
-	for (i = 0; i < m->nr_args; i++)
-		nr_regs += (m->arg_size[i] + 7) / 8 - 1;
-
-	/* x86-64 supports up to MAX_BPF_FUNC_ARGS arguments. 1-6
-	 * are passed through regs, the remains are through stack.
+	/* x86-64 supports up to MAX_BPF_FUNC_ARGS argument slots, an
+	 * argument taking one or two. Six registers hold the first ones
+	 * and the stack the rest, but an argument is never split: one that
+	 * does not fit in the registers left goes to the stack, and a
+	 * later one may still take a register.
 	 */
+	nr_regs = bpf_jit_place_args(&x86_arg_abi, m, pos_of_slot);
 	if (nr_regs > MAX_BPF_FUNC_ARGS)
 		return -ENOTSUPP;
 
@@ -3853,11 +3856,11 @@ static int __arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *rw_im
 	 * RBP - 8         [ return value    ]  BPF_TRAMP_F_CALL_ORIG or
 	 *                                      BPF_TRAMP_F_RET_FENTRY_RET flags
 	 *
-	 *                 [ reg_argN        ]  always
-	 *                 [ ...             ]
-	 * RBP - regs_off  [ reg_arg1        ]  program's ctx pointer
+	 *                 [ arg slotN       ]  always, packed, whether
+	 *                 [ ...             ]  in a register or on the
+	 * RBP - regs_off  [ arg slot1       ]  stack; program's ctx pointer
 	 *
-	 * RBP - func_meta_off [ regs count, etc ]  always
+	 * RBP - func_meta_off [ slot count, etc ]  always
 	 *
 	 * RBP - ip_off    [ traced function ]  BPF_TRAMP_F_IP_ARG flag
 	 *
@@ -3865,10 +3868,10 @@ static int __arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *rw_im
 	 *
 	 * RBP - run_ctx_off [ bpf_tramp_run_ctx ]
 	 *
-	 *                     [ stack_argN ]  BPF_TRAMP_F_CALL_ORIG
-	 *                     [ ...        ]
-	 *                     [ stack_arg2 ]
-	 * RBP - arg_stack_off [ stack_arg1 ]
+	 *                     [ stack slotN ]  BPF_TRAMP_F_CALL_ORIG,
+	 *                     [ ...         ]  the incoming stack
+	 *                     [ stack slot2 ]  arguments, alignment
+	 * RBP - arg_stack_off [ stack slot1 ]  holes included
 	 * RSP                 [ tail_call_cnt_ptr ] BPF_TRAMP_F_TAIL_CALL_CTX
 	 */
 
@@ -3880,7 +3883,7 @@ static int __arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *rw_im
 	stack_size += nr_regs * 8;
 	regs_off = stack_size;
 
-	/* function matedata, such as regs count  */
+	/* function matedata, such as the argument slot count */
 	stack_size += 8;
 	func_meta_off = stack_size;
 
@@ -3902,7 +3905,7 @@ static int __arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *rw_im
 
 	if (nr_regs > 6 && (flags & BPF_TRAMP_F_CALL_ORIG)) {
 		/* the space that used to pass arguments on-stack */
-		stack_size += (nr_regs - get_nr_used_regs(m)) * 8;
+		stack_size += get_nr_stack_slots(pos_of_slot, nr_regs) * 8;
 		/* make sure the stack pointer is 16-byte aligned if we
 		 * need pass arguments on stack, which means
 		 *  [stack_size + 8(rbp) + 8(rip) + 8(origin rip)]
@@ -3962,7 +3965,7 @@ static int __arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *rw_im
 	emit_stx(&prog, BPF_DW, BPF_REG_FP, BPF_REG_6, -rbx_off);
 
 	func_meta = nr_regs;
-	/* Store number of argument registers of the traced function */
+	/* Store number of argument slots of the traced function */
 	emit_store_stack_imm64(&prog, BPF_REG_0, -func_meta_off, func_meta);
 
 	if (flags & BPF_TRAMP_F_IP_ARG) {
@@ -3970,7 +3973,7 @@ static int __arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *rw_im
 		emit_store_stack_imm64(&prog, BPF_REG_0, -ip_off, (long)func_addr);
 	}
 
-	save_args(m, &prog, regs_off, false, flags, arena_base);
+	save_args(m, &prog, pos_of_slot, regs_off, false, flags, arena_base);
 
 	if (flags & BPF_TRAMP_F_CALL_ORIG) {
 		/* arg1: mov rdi, im */
@@ -4011,8 +4014,8 @@ static int __arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *rw_im
 	}
 
 	if (flags & BPF_TRAMP_F_CALL_ORIG) {
-		restore_regs(m, &prog, regs_off);
-		save_args(m, &prog, arg_stack_off, true, flags, 0);
+		restore_regs(&prog, pos_of_slot, nr_regs, regs_off);
+		save_args(m, &prog, pos_of_slot, arg_stack_off, true, flags, 0);
 
 		if (flags & BPF_TRAMP_F_TAIL_CALL_CTX) {
 			/* Before calling the original function, load the
@@ -4065,7 +4068,7 @@ static int __arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *rw_im
 	}
 
 	if (flags & BPF_TRAMP_F_RESTORE_REGS)
-		restore_regs(m, &prog, regs_off);
+		restore_regs(&prog, pos_of_slot, nr_regs, regs_off);
 
 	/* This needs to be done regardless. If there were fmod_ret programs,
 	 * the return value is only updated on the stack and still needs to be
-- 
2.53.0-Meta

[PATCH bpf-next 4/5] selftests/bpf: Test trampoline args around a 16-byte aligned stack arg

[Yonghong Song]

Add two bpf_testmod functions whose 16-byte aligned argument, a struct
holding an __int128, is passed on the stack after a hole:

  - bpf_testmod_test_int128_stack(): seven u64s before the struct, so it
    leaves a register empty on arm64 and a stack slot empty on x86-64.
  - bpf_testmod_test_int128_backfill(): a 16-byte struct of two longs
    moves to the x86-64 stack and gives R9 back to the next argument; the
    struct then leaves a stack slot empty on both x86-64 and arm64.

The __int128 is wrapped in a struct because btf_ctx_access() refuses a
program reading an integer argument wider than eight bytes, while it
lets the program read each slot of a struct argument.

The fentry programs read each argument slot from ctx[] and check that
the trampoline packs them in the BPF convention, and the fexit programs
check the return value, weighted by argument slot, which is only right
if the original function got its arguments back where its convention
wants them.

Without the preceding x86 fix the test fails with:

  test_int128_args:FAIL:s:h.lo unexpected s:h.lo: actual 2 != expected 8
  ...
  test_int128_args:FAIL:s ret unexpected s ret: actual 285 != expected 385

Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
---
 .../selftests/bpf/prog_tests/tracing_struct.c | 22 +++++++--
 .../bpf/progs/tracing_struct_int128.c         | 45 +++++++++++++++++++
 .../selftests/bpf/test_kmods/bpf_testmod.c    | 43 ++++++++++++++++++
 3 files changed, 107 insertions(+), 3 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/tracing_struct.c b/tools/testing/selftests/bpf/prog_tests/tracing_struct.c
index 15b95d0235b5..eced07a61dde 100644
--- a/tools/testing/selftests/bpf/prog_tests/tracing_struct.c
+++ b/tools/testing/selftests/bpf/prog_tests/tracing_struct.c
@@ -117,9 +117,11 @@ static void test_int128_args(void)
 {
 	/*
 	 * __int128 arguments are passed in a register pair on x86_64 and
-	 * arm64, which the trampoline packs into two context slots. Other
-	 * architectures pass a __int128 differently (e.g. s390x passes larger
-	 * arguments by reference), so only exercise this on x86_64 and arm64.
+	 * arm64, which the trampoline packs into two context slots. Past the
+	 * registers, both start an argument aligned to 16 bytes at an even
+	 * stack slot, which the trampoline packs too. Other architectures
+	 * pass a __int128 differently (e.g. s390x passes larger arguments by
+	 * reference), so only exercise this on x86_64 and arm64.
 	 */
 #if defined(__x86_64__) || defined(__aarch64__)
 	struct tracing_struct_int128 *skel;
@@ -139,6 +141,20 @@ static void test_int128_args(void)
 	ASSERT_EQ(skel->bss->t_c, 3, "t:c");
 	ASSERT_EQ(skel->bss->t_ret, 6, "t ret");
 
+	ASSERT_EQ(skel->bss->s_g, 7, "s:g");
+	ASSERT_EQ(skel->bss->s_h_lo, 8, "s:h.lo");
+	ASSERT_EQ(skel->bss->s_h_hi, 9, "s:h.hi");
+	ASSERT_EQ(skel->bss->s_i, 10, "s:i");
+	ASSERT_EQ(skel->bss->s_ret, 385, "s ret");
+
+	ASSERT_EQ(skel->bss->b_f_a, 6, "b:f.a");
+	ASSERT_EQ(skel->bss->b_f_b, 7, "b:f.b");
+	ASSERT_EQ(skel->bss->b_g, 8, "b:g");
+	ASSERT_EQ(skel->bss->b_h, 9, "b:h");
+	ASSERT_EQ(skel->bss->b_i_lo, 10, "b:i.lo");
+	ASSERT_EQ(skel->bss->b_i_hi, 11, "b:i.hi");
+	ASSERT_EQ(skel->bss->b_ret, 506, "b ret");
+
 destroy_skel:
 	tracing_struct_int128__destroy(skel);
 #else
diff --git a/tools/testing/selftests/bpf/progs/tracing_struct_int128.c b/tools/testing/selftests/bpf/progs/tracing_struct_int128.c
index 4638dfec1f38..5520ee500c01 100644
--- a/tools/testing/selftests/bpf/progs/tracing_struct_int128.c
+++ b/tools/testing/selftests/bpf/progs/tracing_struct_int128.c
@@ -15,4 +15,49 @@ int test_int128_arg_fexit(unsigned long long *ctx)
 	return 0;
 }
 
+/*
+ * The arguments come packed in the BPF convention whatever holes the
+ * kernel convention left between them: an eightbyte each, and two for the
+ * struct holding an __int128.
+ */
+long s_g, s_h_lo, s_h_hi, s_i, s_ret;
+
+SEC("fentry/bpf_testmod_test_int128_stack")
+int test_int128_stack_fentry(unsigned long long *ctx)
+{
+	s_g = ctx[6];
+	s_h_lo = ctx[7];
+	s_h_hi = ctx[8];
+	s_i = ctx[9];
+	return 0;
+}
+
+SEC("fexit/bpf_testmod_test_int128_stack")
+int test_int128_stack_fexit(unsigned long long *ctx)
+{
+	s_ret = ctx[10];
+	return 0;
+}
+
+long b_f_a, b_f_b, b_g, b_h, b_i_lo, b_i_hi, b_ret;
+
+SEC("fentry/bpf_testmod_test_int128_backfill")
+int test_int128_backfill_fentry(unsigned long long *ctx)
+{
+	b_f_a = ctx[5];
+	b_f_b = ctx[6];
+	b_g = ctx[7];
+	b_h = ctx[8];
+	b_i_lo = ctx[9];
+	b_i_hi = ctx[10];
+	return 0;
+}
+
+SEC("fexit/bpf_testmod_test_int128_backfill")
+int test_int128_backfill_fexit(unsigned long long *ctx)
+{
+	b_ret = ctx[11];
+	return 0;
+}
+
 char _license[] SEC("license") = "GPL";
diff --git a/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c b/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c
index 54a5fc39e0e2..23a2c88d3e7c 100644
--- a/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c
+++ b/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c
@@ -197,6 +197,43 @@ bpf_testmod_test_int128_arg(__int128 a, int b, long c)
 	bpf_testmod_test_struct_arg_result = (long)a + b + c;
 	return bpf_testmod_test_struct_arg_result;
 }
+
+struct bpf_testmod_struct_int128 {
+	__int128 a;
+};
+
+/*
+ * x86-64 puts g in the first stack slot and h, aligned to 16 bytes, in
+ * the third and fourth, leaving a hole. arm64 leaves x7 empty and puts h
+ * in the first two stack slots. Either way h and i are not where the BPF
+ * convention packs them, right after g.
+ */
+noinline long
+bpf_testmod_test_int128_stack(u64 a, u64 b, u64 c, u64 d, u64 e, u64 f,
+			      u64 g, struct bpf_testmod_struct_int128 h, u64 i)
+{
+	bpf_testmod_test_struct_arg_result = a + b * 2 + c * 3 + d * 4 + e * 5 +
+		f * 6 + g * 7 + (u64)h.a * 8 +
+		(u64)((unsigned __int128)h.a >> 64) * 9 + i * 10;
+	return bpf_testmod_test_struct_arg_result;
+}
+
+/*
+ * x86-64 has no register left for f after e, so f goes to the stack and g
+ * takes R9 back; h goes to the third stack slot and i, aligned to 16
+ * bytes, to the fifth and sixth. arm64 has registers up to g, puts h in
+ * the first stack slot and i in the third and fourth.
+ */
+noinline long
+bpf_testmod_test_int128_backfill(u64 a, u64 b, u64 c, u64 d, u64 e,
+				 struct bpf_testmod_struct_arg_2 f, u64 g, u64 h,
+				 struct bpf_testmod_struct_int128 i)
+{
+	bpf_testmod_test_struct_arg_result = a + b * 2 + c * 3 + d * 4 + e * 5 +
+		f.a * 6 + f.b * 7 + g * 8 + h * 9 + (u64)i.a * 10 +
+		(u64)((unsigned __int128)i.a >> 64) * 11;
+	return bpf_testmod_test_struct_arg_result;
+}
 #endif
 
 __weak noinline void bpf_testmod_looooooooooooooooooooooooooooooong_name(void)
@@ -683,6 +720,12 @@ bpf_testmod_test_read(struct file *file, struct kobject *kobj,
 #ifdef __SIZEOF_INT128__
 	(void)bpf_testmod_test_int128_ret(i);
 	(void)bpf_testmod_test_int128_arg((__int128)1, 2, 3);
+	(void)bpf_testmod_test_int128_stack(1, 2, 3, 4, 5, 6, 7,
+		(struct bpf_testmod_struct_int128){ .a = ((__int128)9 << 64) | 8 },
+		10);
+	(void)bpf_testmod_test_int128_backfill(1, 2, 3, 4, 5,
+		(struct bpf_testmod_struct_arg_2){ .a = 6, .b = 7 }, 8, 9,
+		(struct bpf_testmod_struct_int128){ .a = ((__int128)11 << 64) | 10 });
 #endif
 
 	(void)trace_bpf_testmod_test_raw_tp_null_tp(NULL);
-- 
2.53.0-Meta

[PATCH bpf-next 5/5] selftests/bpf: Test trampoline args of a BPF function at an odd slot

[Yonghong Song]

Attach fentry and fexit programs to two functions of another BPF
program, each taking a struct holding an __int128 at an odd argument
slot:

  - callee_odd(): the struct after a single u64, where arm64 would
    leave a register empty.
  - callee_stack(): the struct after seven u64s, where x86-64 would
    leave a stack slot empty and arm64 a register.

BPF code calls them with the arguments packed in the BPF convention, so
that is where the trampoline has to take them from. The fentry programs
read each argument slot from ctx[], and the fexit programs check the
return value, weighted by slot, which is only right if the function got
its arguments back where its caller put them. Both take a struct by
value, which GCC passes by reference instead, so they are only built
with clang, and callee_stack() only with a clang that supports stack
arguments.

Placing a BPF function's arguments by the kernel convention instead, the
x86-64 trampoline fails the test with:

  test_int128_tgt_prog:FAIL:stack:h.lo unexpected stack:h.lo: actual 9 != expected 8
  ...
  test_int128_tgt_prog:FAIL:stack fexit ret unexpected stack fexit ret: actual 34359738697 != expected 385

Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
---
 .../selftests/bpf/prog_tests/tracing_struct.c | 74 +++++++++++++++++++
 .../bpf/progs/tracing_struct_int128_tgt.c     | 68 +++++++++++++++++
 .../bpf/progs/tracing_struct_int128_tracer.c  | 44 +++++++++++
 3 files changed, 186 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/progs/tracing_struct_int128_tgt.c
 create mode 100644 tools/testing/selftests/bpf/progs/tracing_struct_int128_tracer.c

diff --git a/tools/testing/selftests/bpf/prog_tests/tracing_struct.c b/tools/testing/selftests/bpf/prog_tests/tracing_struct.c
index eced07a61dde..d0bdd2a72b0b 100644
--- a/tools/testing/selftests/bpf/prog_tests/tracing_struct.c
+++ b/tools/testing/selftests/bpf/prog_tests/tracing_struct.c
@@ -5,6 +5,8 @@
 #include "tracing_struct.skel.h"
 #include "tracing_struct_many_args.skel.h"
 #include "tracing_struct_int128.skel.h"
+#include "tracing_struct_int128_tgt.skel.h"
+#include "tracing_struct_int128_tracer.skel.h"
 
 static void test_struct_args(void)
 {
@@ -162,6 +164,76 @@ static void test_int128_args(void)
 #endif
 }
 
+static void test_int128_tgt_prog(void)
+{
+#if defined(__x86_64__) || defined(__aarch64__)
+	struct tracing_struct_int128_tracer *tracer = NULL;
+	struct tracing_struct_int128_tgt *tgt;
+	LIBBPF_OPTS(bpf_test_run_opts, topts);
+	int err, fd;
+
+	tgt = tracing_struct_int128_tgt__open_and_load();
+	if (!ASSERT_OK_PTR(tgt, "tgt__open_and_load"))
+		return;
+	if (!tgt->rodata->has_struct_arg) {
+		test__skip();
+		goto out;
+	}
+	fd = bpf_program__fd(tgt->progs.run);
+
+	tracer = tracing_struct_int128_tracer__open();
+	if (!ASSERT_OK_PTR(tracer, "tracer__open"))
+		goto out;
+
+	err = bpf_program__set_attach_target(tracer->progs.odd_entry, fd, "callee_odd");
+	err = err ?: bpf_program__set_attach_target(tracer->progs.odd_exit, fd, "callee_odd");
+	if (tgt->rodata->has_stack_arg) {
+		err = err ?: bpf_program__set_attach_target(tracer->progs.stack_entry, fd,
+							     "callee_stack");
+		err = err ?: bpf_program__set_attach_target(tracer->progs.stack_exit, fd,
+							     "callee_stack");
+	} else {
+		bpf_program__set_autoload(tracer->progs.stack_entry, false);
+		bpf_program__set_autoload(tracer->progs.stack_exit, false);
+	}
+	if (!ASSERT_OK(err, "set_attach_target"))
+		goto out;
+
+	err = tracing_struct_int128_tracer__load(tracer);
+	if (!ASSERT_OK(err, "tracer__load"))
+		goto out;
+	err = tracing_struct_int128_tracer__attach(tracer);
+	if (!ASSERT_OK(err, "tracer__attach"))
+		goto out;
+
+	err = bpf_prog_test_run_opts(fd, &topts);
+	if (!ASSERT_OK(err, "test_run") || !ASSERT_EQ(topts.retval, 0, "retval"))
+		goto out;
+
+	ASSERT_EQ(tracer->bss->odd_a, 1, "odd:a");
+	ASSERT_EQ(tracer->bss->odd_lo, 8, "odd:v.lo");
+	ASSERT_EQ(tracer->bss->odd_hi, 9, "odd:v.hi");
+	ASSERT_EQ(tracer->bss->odd_b, 2, "odd:b");
+	ASSERT_EQ(tgt->bss->ret_odd, 52, "odd ret");
+	ASSERT_EQ(tracer->bss->odd_ret, 52, "odd fexit ret");
+
+	if (tgt->rodata->has_stack_arg) {
+		ASSERT_EQ(tracer->bss->stack_g, 7, "stack:g");
+		ASSERT_EQ(tracer->bss->stack_lo, 8, "stack:h.lo");
+		ASSERT_EQ(tracer->bss->stack_hi, 9, "stack:h.hi");
+		ASSERT_EQ(tracer->bss->stack_i, 10, "stack:i");
+		ASSERT_EQ(tgt->bss->ret_stack, 385, "stack ret");
+		ASSERT_EQ(tracer->bss->stack_ret, 385, "stack fexit ret");
+	}
+
+out:
+	tracing_struct_int128_tracer__destroy(tracer);
+	tracing_struct_int128_tgt__destroy(tgt);
+#else
+	test__skip();
+#endif
+}
+
 static void test_union_args(void)
 {
 	struct tracing_struct *skel;
@@ -197,6 +269,8 @@ void test_tracing_struct(void)
 		test_struct_many_args();
 	if (test__start_subtest("int128_args"))
 		test_int128_args();
+	if (test__start_subtest("int128_tgt_prog"))
+		test_int128_tgt_prog();
 	if (test__start_subtest("union_args"))
 		test_union_args();
 }
diff --git a/tools/testing/selftests/bpf/progs/tracing_struct_int128_tgt.c b/tools/testing/selftests/bpf/progs/tracing_struct_int128_tgt.c
new file mode 100644
index 000000000000..2ec8f741d48b
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/tracing_struct_int128_tgt.c
@@ -0,0 +1,68 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2026 Meta Platforms, Inc. and affiliates. */
+#include <vmlinux.h>
+#include <bpf/bpf_helpers.h>
+
+struct int128_arg {
+	__int128 v;
+};
+
+__u64 base;
+__u64 ret_odd, ret_stack;
+
+#if defined(__clang__)
+
+const volatile bool has_struct_arg = true;
+
+/* arm64 would want v in x2 and x3, and b in x4. */
+__noinline __u64 callee_odd(__u64 a, struct int128_arg v, __u64 b)
+{
+	return a + (__u64)v.v * 2 + (__u64)((unsigned __int128)v.v >> 64) * 3 +
+	       b * 4;
+}
+
+#else
+
+const volatile bool has_struct_arg = false;
+
+#endif
+
+#if defined(__clang__) && defined(__BPF_FEATURE_STACK_ARGUMENT)
+
+const volatile bool has_stack_arg = true;
+
+/*
+ * x86-64 would want h in its third and fourth stack slots, and arm64 in
+ * its first and second, leaving x7 empty.
+ */
+static __noinline __u64 callee_stack(__u64 a, __u64 b, __u64 c, __u64 d,
+				     __u64 e, __u64 f, __u64 g,
+				     struct int128_arg h, __u64 i)
+{
+	return a + b * 2 + c * 3 + d * 4 + e * 5 + f * 6 + g * 7 +
+	       (__u64)h.v * 8 + (__u64)((unsigned __int128)h.v >> 64) * 9 +
+	       i * 10;
+}
+
+#else
+
+const volatile bool has_stack_arg = false;
+
+#endif
+
+SEC("syscall")
+int run(void *ctx)
+{
+#if defined(__clang__)
+	struct int128_arg v = { .v = ((__int128)(base + 9) << 64) | (base + 8) };
+
+	ret_odd = callee_odd(base + 1, v, base + 2);
+#if defined(__BPF_FEATURE_STACK_ARGUMENT)
+	ret_stack = callee_stack(base + 1, base + 2, base + 3, base + 4,
+				 base + 5, base + 6, base + 7, v, base + 10);
+#endif
+#endif
+	return 0;
+}
+
+char _license[] SEC("license") = "GPL";
diff --git a/tools/testing/selftests/bpf/progs/tracing_struct_int128_tracer.c b/tools/testing/selftests/bpf/progs/tracing_struct_int128_tracer.c
new file mode 100644
index 000000000000..133f24fc9158
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/tracing_struct_int128_tracer.c
@@ -0,0 +1,44 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2026 Meta Platforms, Inc. and affiliates. */
+#include <vmlinux.h>
+#include <bpf/bpf_helpers.h>
+
+long odd_a, odd_lo, odd_hi, odd_b, odd_ret;
+
+SEC("fentry/callee_odd")
+int odd_entry(unsigned long long *ctx)
+{
+	odd_a = ctx[0];
+	odd_lo = ctx[1];
+	odd_hi = ctx[2];
+	odd_b = ctx[3];
+	return 0;
+}
+
+SEC("fexit/callee_odd")
+int odd_exit(unsigned long long *ctx)
+{
+	odd_ret = ctx[4];
+	return 0;
+}
+
+long stack_g, stack_lo, stack_hi, stack_i, stack_ret;
+
+SEC("fentry/callee_stack")
+int stack_entry(unsigned long long *ctx)
+{
+	stack_g = ctx[6];
+	stack_lo = ctx[7];
+	stack_hi = ctx[8];
+	stack_i = ctx[9];
+	return 0;
+}
+
+SEC("fexit/callee_stack")
+int stack_exit(unsigned long long *ctx)
+{
+	stack_ret = ctx[10];
+	return 0;
+}
+
+char _license[] SEC("license") = "GPL";
-- 
2.53.0-Meta

Re: [PATCH bpf-next 4/5] selftests/bpf: Test trampoline args around a 16-byte aligned stack arg

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] bpf: endianness mismatch when reading __int128 in selftests on big-endian
- [Medium] selftests/bpf: data race on bpf_testmod_test_struct_arg_result
--

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

selftests/bpf: Test trampoline args around a 16-byte aligned stack arg

This commit introduces two new bpf_testmod functions to test trampoline
argument handling for 16-byte aligned stack arguments, specifically targeting
__int128 structures. It also adds fentry and fexit programs to verify that
arguments are correctly packed in the context array according to BPF
conventions on x86-64 and arm64.

> diff --git a/tools/testing/selftests/bpf/progs/tracing_struct_int128.c b/tools/testing/selftests/bpf/progs/tracing_struct_int128.c
> index 4638dfec1f38e..5520ee500c01d 100644
> --- a/tools/testing/selftests/bpf/progs/tracing_struct_int128.c
> +++ b/tools/testing/selftests/bpf/progs/tracing_struct_int128.c
> @@ -15,4 +15,49 @@ int test_int128_arg_fexit(unsigned long long *ctx)
>  	return 0;
>  }
>  
> +/*
> + * The arguments come packed in the BPF convention whatever holes the
> + * kernel convention left between them: an eightbyte each, and two for the
> + * struct holding an __int128.
> + */
> +long s_g, s_h_lo, s_h_hi, s_i, s_ret;
> +
> +SEC("fentry/bpf_testmod_test_int128_stack")
> +int test_int128_stack_fentry(unsigned long long *ctx)
> +{
> +	s_g = ctx[6];
> +	s_h_lo = ctx[7];
> +	s_h_hi = ctx[8];
[Severity: Medium]
Will this little-endian assumption cause the selftest to fail on big-endian
systems? On big-endian arm64 (aarch64_be), the high 64 bits of the 16-byte
struct would be at the lower memory address, so reading ctx[7] as the low
64 bits and ctx[8] as the high 64 bits might be reversed.

> +	s_i = ctx[9];
> +	return 0;
> +}
> +

[ ... ]

> +long b_f_a, b_f_b, b_g, b_h, b_i_lo, b_i_hi, b_ret;
> +
> +SEC("fentry/bpf_testmod_test_int128_backfill")
> +int test_int128_backfill_fentry(unsigned long long *ctx)
> +{
> +	b_f_a = ctx[5];
> +	b_f_b = ctx[6];
> +	b_g = ctx[7];
> +	b_h = ctx[8];
> +	b_i_lo = ctx[9];
> +	b_i_hi = ctx[10];
[Severity: Medium]
Does this have the same endianness mismatch on big-endian architectures?
Extracting the 16-byte struct here from ctx[9] and ctx[10] also seems to
assume a little-endian memory layout.

> +	return 0;
> +}

> diff --git a/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c b/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c
> index 54a5fc39e0e27..23a2c88d3e7c1 100644
> --- a/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c
> +++ b/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c

[ ... ]

> +noinline long
> +bpf_testmod_test_int128_stack(u64 a, u64 b, u64 c, u64 d, u64 e, u64 f,
> +			      u64 g, struct bpf_testmod_struct_int128 h, u64 i)
> +{
> +	bpf_testmod_test_struct_arg_result = a + b * 2 + c * 3 + d * 4 + e * 5 +
> +		f * 6 + g * 7 + (u64)h.a * 8 +
> +		(u64)((unsigned __int128)h.a >> 64) * 9 + i * 10;
> +	return bpf_testmod_test_struct_arg_result;
> +}
[Severity: Medium]
Can concurrent execution of tests (like test_progs -j) lead to a data race on
this global variable? If one test thread computes the result in
bpf_testmod_test_int128_stack() and another thread overwrites
bpf_testmod_test_struct_arg_result in a different function before the first
returns, it could cause the BPF fexit hook to read an incorrect return value
into s_ret, leading to flaky ASSERT_EQ failures.

> +
> +/*
> + * x86-64 has no register left for f after e, so f goes to the stack and g
> + * takes R9 back; h goes to the third stack slot and i, aligned to 16
> + * bytes, to the fifth and sixth. arm64 has registers up to g, puts h in
> + * the first stack slot and i in the third and fourth.
> + */
> +noinline long
> +bpf_testmod_test_int128_backfill(u64 a, u64 b, u64 c, u64 d, u64 e,
> +				 struct bpf_testmod_struct_arg_2 f, u64 g, u64 h,
> +				 struct bpf_testmod_struct_int128 i)
> +{
> +	bpf_testmod_test_struct_arg_result = a + b * 2 + c * 3 + d * 4 + e * 5 +
> +		f.a * 6 + f.b * 7 + g * 8 + h * 9 + (u64)i.a * 10 +
> +		(u64)((unsigned __int128)i.a >> 64) * 11;
> +	return bpf_testmod_test_struct_arg_result;
[Severity: Medium]
Since this function also writes to the unprotected global variable
bpf_testmod_test_struct_arg_result and immediately returns it, could this
contribute to the same data race when multiple test processes run
trigger_module_test_read() concurrently?

> +}
>  #endif

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

Re: [PATCH bpf-next 3/5] bpf, x86: Place trampoline arguments by the x86-64 calling convention

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] bpf, x86: stack arguments offset mismatch and misalignment with BPF_TRAMP_F_TAIL_CALL_CTX
- [Medium] bpf: info leak of uninitialized kernel stack memory due to missing garbage cleanup
--

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

bpf, x86: Place trampoline arguments by the x86-64 calling convention

This commit updates the BPF trampoline to place on-stack arguments
according to the x86-64 calling convention. It introduces a backfill
mechanism using bpf_jit_place_args() to handle arguments like __int128
that may span stack slots and create alignment holes.

> diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
> index 083fcd6cf15b7..0fc7715e569b5 100644
> --- a/arch/x86/net/bpf_jit_comp.c
> +++ b/arch/x86/net/bpf_jit_comp.c
[ ... ]
> @@ -3472,8 +3479,35 @@ static void save_args(const struct btf_func_model *m, u8 **prog,
>  		 *
>  		 * the arg1-5,arg7 will be passed by regs, and arg6 will
>  		 * by stack.
> +		 *
> +		 * An argument aligned to 16 bytes, an __int128 or a struct
> +		 * holding one, also starts at an even stack slot, which can
> +		 * leave a hole before it. bpf_jit_place_args() worked out
> +		 * where each eightbyte is.
>  		 */
> -		if (nr_regs + arg_regs > 6) {
> +		for (j = 0; j < arg_regs; j++, slot++, bargs_off -= 8) {
> +			u8 pos = pos_of_slot[slot];
> +			u32 src;
> +
> +			if (!x86_arg_on_stack(pos)) {
[ ... ]
> +			/* copy function arguments from origin stack frame
> +			 * into current stack frame.
[ ... ]
> +			 *
> +			 * The area for the origin call mirrors the incoming
> +			 * one, hole and all; only the bpf program takes the
> +			 * arguments packed.
>  			 */
> -			for (j = 0; j < arg_regs; j++) {
> -				emit_ldx(prog, BPF_DW, BPF_REG_0, BPF_REG_FP,
> -					 nr_stack_slots * 8 + stack_args_off);
> -				if (arena_arg)
> -					emit_arena_arg_conv(prog, BPF_REG_0, nullable,
> -							    (u32)arena_base);
> -				emit_stx(prog, BPF_DW, BPF_REG_FP, BPF_REG_0,
> -					 -stack_size);
> -
> -				if (!nr_stack_slots)
> -					first_off = stack_size;
> -				stack_size -= 8;
> -				nr_stack_slots++;
> -			}
> +			off = x86_arg_stack_off(pos);
> +			doff = for_call_origin ? stack_size - off : bargs_off;
> +			emit_ldx(prog, BPF_DW, BPF_REG_0, BPF_REG_FP,
> +				 stack_args_off + off);
> +			if (arena_arg)
> +				emit_arena_arg_conv(prog, BPF_REG_0, nullable,
> +						    (u32)arena_base);
> +			emit_stx(prog, BPF_DW, BPF_REG_FP, BPF_REG_0, -doff);
> +
> +			if (!nr_stack_slots)
> +				first_off = doff;
> +			nr_stack_slots++;
>  		}
>  	}
>  
>  	clean_stack_garbage(m, prog, nr_stack_slots, first_off);
>  }

[Severity: Medium]
Does this code leak uninitialized kernel stack memory?

The x86-64 ABI allocates 8-byte stack slots for arguments, but compilers
frequently use 32-bit instructions to place 32-bit arguments, leaving the upper
4 bytes of the slot uninitialized.

Since clean_stack_garbage() explicitly returns early when nr_stack_slots != 1,
copying the full 64 bits with emit_ldx(..., BPF_DW, ...) captures this
uninitialized kernel memory into the BPF context. Can this leak kernel stack
memory to privileged BPF users if there are multiple stack arguments?

[ ... ]
> @@ -4011,8 +4014,8 @@ static int __arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *rw_im
>  	}
>  
>  	if (flags & BPF_TRAMP_F_CALL_ORIG) {
> -		restore_regs(m, &prog, regs_off);
> -		save_args(m, &prog, arg_stack_off, true, flags, 0);
> +		restore_regs(&prog, pos_of_slot, nr_regs, regs_off);
> +		save_args(m, &prog, pos_of_slot, arg_stack_off, true, flags, 0);
>  
>  		if (flags & BPF_TRAMP_F_TAIL_CALL_CTX) {
>  			/* Before calling the original function, load the

[Severity: High]
Are stack arguments misaligned here if BPF_TRAMP_F_TAIL_CALL_CTX is active?

When __arch_prepare_bpf_trampoline() executes, it checks for
BPF_TRAMP_F_TAIL_CALL_CTX and emits a push rax instruction (pushing 8 bytes)
after sub rsp, stack_size, which permanently shifts RSP down by 8 bytes.

However, the updated save_args() calculates the stack argument offsets based on
the original stack_size (doff = stack_size - off) without accounting for this
extra 8-byte shift.

Because the push rax is unpopped before call orig_call, the original function
will read its first stack argument from [RSP + 8]. Due to the 8-byte shift,
this points to the tail call counter pointer instead of the actual first stack
argument, corrupting all on-stack arguments.

Additionally, doesn't this 8-byte shift break the required 16-byte stack
alignment on function entry for x86-64?

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

Re: [PATCH bpf-next 2/5] bpf: Keep the BPF calling convention for a trampoline on a BPF function

[bot+bpf-ci]

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

> bpf: Keep the BPF calling convention for a trampoline on a BPF function

TL;DR: On arm64, the identity slot mapping for BPF targets may split a 16-byte
argument across x7 and the stack; fexit/fentry could then read x8 or
uninitialised stack, corrupting the traced function's values.

> diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
> --- a/kernel/bpf/core.c
> +++ b/kernel/bpf/core.c

[ ... ]

> @@ -3342,6 +3342,18 @@ u32 bpf_jit_place_args(const struct bpf_jit_arg_abi *abi,
>  	u32 i, k, nslots, slot = 0, nregs_used = 0, stack_off = 0;
>  	bool on_stack = false;
>
> +	/*
> +	 * A BPF function is called by BPF code, which passes slot N where
> +	 * the kernel convention passes its Nth eightbyte, whatever the
> +	 * arguments: no hole, no split refused and no register backfilled.
> +	 */
> +	if (fm->bpf_tgt_prog) {
> +		for (i = 0; i < fm->nr_args; i++)
> +			for (k = btf_func_model_arg_slots(fm, i); k > 0; k--, slot++)
> +				pos_of_slot[slot] = slot;
> +		return slot;
> +	}
> +

Can this identity mapping split one 16-byte argument across the last
register and the stack on arm64, which is the only trampoline calling
bpf_jit_place_args() at this commit?

Before this patch, arm64_arg_abi has no split_at_boundary, so every slot of
an argument landed either in x0-x7 or on the stack.  calc_arg_aux() relies
on that and only checks the first slot of each argument:

    for (i = 0, slot = 0; i < m->nr_args; i++) {
        slots = (m->arg_size[i] + 7) / 8;
        if (a64_arg_on_stack(a->pos_of_slot[slot])) /* passed through register ? */
            break;
        slot += slots;
    }

save_args() then uses each pos_of_slot value of such an "in register"
argument as a register number:

    reg = a->pos_of_slot[slot++];
    if (for_call_origin) {
        emit(A64_LDR64I(reg, A64_SP, bargs_off), ctx);
    ...
    } else {
        emit(A64_STR64I(reg, A64_SP, bargs_off), ctx);
    }

With the identity mapping, a 16-byte argument at BPF slot 7 gets
pos_of_slot = {7, 8}.  This looks reachable: a static subprog can have up
to 12 argument slots (btf_check_arg_slots(), and
bpf_jit_supports_stack_args() is true on arm64), and btf_prepare_func_args()
accepts scalar-only structs up to 16 bytes.  The selftest added later in
this series (e5dd7e2f7244) has such a function:

    static __noinline __u64 callee_stack(__u64 a, __u64 b, __u64 c, __u64 d,
                                         __u64 e, __u64 f, __u64 g,
                                         struct int128_arg h, __u64 i)

The arm64 JIT passes h.lo in x7 (stack_arg_reg[2]) and h.hi at [SP+0]
(emit_stack_arg_store(), idx 3).  calc_arg_aux() then returns
args_in_regs = 8, regs_for_args = 9, bstack_for_args = 8 and
ostack_for_args = 16, so:

- fentry: save_args() stores A64_R(8) into ctx[8].  On arm64, x8 is
  BPF_REG_0 of the calling BPF code (bpf2a64[BPF_REG_0] = A64_R(8)), not
  h.hi, which is still at FP + 32 + 0.

- fexit (BPF_TRAMP_F_CALL_ORIG): save_args(..., true, ...) loads the saved
  value into x8 and never writes oargs_off + 0.  The original function then
  reads h.hi from uninitialised trampoline stack.  If the split argument is
  the last one, bstack_for_args and ostack_for_args are both 0, and the
  function reads [SP+0] of the trampoline frame, which is the
  bpf_tramp_run_ctx.

Attaching fexit would then change the values the traced BPF function
computes with, and what it returns.

The new comment says the BPF convention has "no split refused", and the x86
follow-up f5df67b558e5 ("bpf, x86: Place trampoline arguments by the x86-64
calling convention") handles a split per slot for x86.  But no commit in the
range updates arch/arm64/net/bpf_jit_comp.c.  The int128_tgt_prog selftest
in e5dd7e2f7244 also runs on __aarch64__, where it would expect ctx[8] == 9
but read x8.

Should calc_arg_aux(), save_args() and restore_args() on arm64 classify and
read each slot by its own position (register vs. FP + 32 +
a64_arg_stack_off()), the way the x86 follow-up does, so that a split
16-byte argument works?


---
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/37897160282

Re: [PATCH bpf-next 1/5] selftests/bpf: Test by-value __int128 kfunc arguments at an odd slot

[bot+bpf-ci]

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

> selftests/bpf: Test by-value __int128 kfunc arguments at an odd slot

TL;DR: On arm64 with an older pahole, the two new kfuncs lack BTF, so
bpf_testmod.ko may fail to build or load, losing all bpf_testmod selftests,
not just the new ones.

> On arm64 the first two kfuncs get BTF only with the pahole patch
> "dwarf_loader: Skip the argument register the arm64 ABI leaves as a hole"
> [2], in pahole master after v1.32. An older pahole finds the argument
> after the __int128 in an unexpected register, x4 rather than x3, and
> leaves the function out of BTF, so the tests fail to load there.

> diff --git a/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c b/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c
> --- a/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c
> +++ b/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c

[ ... ]

> @@ -1763,6 +1786,9 @@ 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_i128_arg_odd)
> +BTF_ID_FLAGS(func, bpf_kfunc_call_test_i128_arg_odd_many)
> +BTF_ID_FLAGS(func, bpf_kfunc_call_test_i128_arg_stack_hole)
>  BTF_ID_FLAGS(func, bpf_kfunc_call_test_i128_arg_pad)

Is the failure on arm64 with an older pahole really limited to the two new
tests?

Both bpf_kfunc_call_test_i128_arg_odd and
bpf_kfunc_call_test_i128_arg_odd_many are added to
bpf_testmod_check_kfunc_ids, the module's main kfunc set.

With no BTF FUNC for them, resolve_btfids only warns ("no BTF func for
kfunc ..." in collect_kfuncs() and "unresolved symbol ..." in id_patch())
and leaves their set8 ids at 0.  With CONFIG_WERROR=y, scripts/Makefile.btf
passes --fatal_warnings, so bpf_testmod.ko would fail to build.

Otherwise, at module load bpf_testmod_init() calls
register_btf_kfunc_id_set(BPF_PROG_TYPE_SCHED_CLS, &bpf_testmod_kfunc_set),
and __register_btf_kfunc_id_set() checks every entry:

    for (i = 0; i < kset->set->cnt; i++) {
        ret = btf_check_kfunc_protos(btf, btf_relocate_id(btf, kset->set->pairs[i].id), ...);
        if (ret)
            goto err_out;
    }

btf_check_kfunc_protos() does:

    func = btf_type_by_id(btf, func_id);
    if (!func || !btf_type_is_func(func))
        return -EINVAL;

Id 0 resolves to btf_void in the base BTF (btf_relocate_id() maps 0 to 0),
so this returns -EINVAL.

That would reject the whole set, bpf_testmod_init() would return the error,
and bpf_testmod.ko would not load.  test_progs then prints "Selftests
relying on bpf_testmod.ko will be skipped", so every bpf_testmod-dependent
selftest on arm64 would be lost, not only the two new ones.  The same
configuration worked before this patch.

Could the two arm64-hole kfuncs go into a separate kfunc set whose
registration failure is tolerated?  Alternatively, should the commit message
say that bpf_testmod.ko as a whole needs the newer pahole on arm64, rather
than "the tests fail to load there"?


---
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/37897160282

Re: [PATCH bpf-next 5/5] selftests/bpf: Test trampoline args of a BPF function at an odd slot

[bot+bpf-ci]

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

> selftests/bpf: Test trampoline args of a BPF function at an odd slot

TL;DR:
- On arm64, the callee_stack case may read an uninitialized stack value for
  h.hi, so its assertions may fail
- Silent test__skip() without a message (nit, matches existing idiom)

> diff --git a/tools/testing/selftests/bpf/prog_tests/tracing_struct.c b/tools/testing/selftests/bpf/prog_tests/tracing_struct.c
> --- a/tools/testing/selftests/bpf/prog_tests/tracing_struct.c
> +++ b/tools/testing/selftests/bpf/prog_tests/tracing_struct.c

[ ... ]

> @@ -162,6 +164,76 @@ static void test_int128_args(void)
>  #endif
>  }
>
> +static void test_int128_tgt_prog(void)
> +{
> +#if defined(__x86_64__) || defined(__aarch64__)
> +	struct tracing_struct_int128_tracer *tracer = NULL;
> +	struct tracing_struct_int128_tgt *tgt;
> +	LIBBPF_OPTS(bpf_test_run_opts, topts);
> +	int err, fd;
> +
> +	tgt = tracing_struct_int128_tgt__open_and_load();
> +	if (!ASSERT_OK_PTR(tgt, "tgt__open_and_load"))
> +		return;
> +	if (!tgt->rodata->has_struct_arg) {
> +		test__skip();
> +		goto out;

A subsystem pattern flags this as potentially concerning, though this isn't
a bug: test__skip() is called here without any message, and the #else branch
for other architectures does the same.  When the target object was built
without by-value struct argument support (a GCC BPF build), the run shows a
bare SKIP with nothing explaining what was missing.

The pattern says a test that skips silently with no message should be
reported.  test__skip() takes no message argument, and test_int128_args()
and prog_tests/stack_arg.c use the same bare test__skip() call, so this may
simply match the existing idiom.

Would a printf() of the reason before test__skip() be worth adding here?

> +	}
> +	fd = bpf_program__fd(tgt->progs.run);

[ ... ]

> +	if (tgt->rodata->has_stack_arg) {
> +		ASSERT_EQ(tracer->bss->stack_g, 7, "stack:g");
> +		ASSERT_EQ(tracer->bss->stack_lo, 8, "stack:h.lo");
> +		ASSERT_EQ(tracer->bss->stack_hi, 9, "stack:h.hi");

Does the callee_stack half of this subtest pass on arm64?

In the BPF convention, callee_stack(a, b, c, d, e, f, g, struct int128_arg h,
i) uses argument slots 0..6 for a..g, 7..8 for h and 9 for i.  The arm64 JIT
passes BPF slots 5..7 in x5..x7 and slot 8 onward on the stack, so h.lo is in
x7 and h.hi is at [SP + 0].

For a BPF target, commit 0ee21f37b2cb ("bpf: Keep the BPF calling convention
for a trampoline on a BPF function") makes bpf_jit_place_args() return the
identity mapping:

    if (fm->bpf_tgt_prog) {
        for (i = 0; i < fm->nr_args; i++)
            for (k = btf_func_model_arg_slots(fm, i); k > 0; k--, slot++)
                pos_of_slot[slot] = slot;

The arm64 code decides whether an argument is in registers or on the stack
from its first slot only, in calc_arg_aux():

    if (a64_arg_on_stack(a->pos_of_slot[slot])) /* passed through register ? */
        break;
    slot += slots;

h starts at pos 7, so it is counted as a register argument (args_in_regs = 8,
regs_for_args = 9).  Then save_args() handles h's second slot with:

    reg = a->pos_of_slot[slot++];
    ...
    emit(A64_STR64I(reg, A64_SP, bargs_off), ctx);

with reg = 8.  That stores x8 (BPF R0 in bpf2a64[]) into ctx[8] instead of
reading h.hi from the stack.  On the call-origin path the same loop reloads
x8, and the stack loop writes only i, at oargs_off + 8.  The outgoing slot 0
is never written, so callee_stack() would read h.hi from uninitialized
trampoline stack.

With a clang that defines __BPF_FEATURE_STACK_ARGUMENT, should the
"stack:h.hi", "stack ret" and "stack fexit ret" assertions be expected to
fail on arm64?  A leftover kernel stack value would also end up in the traced
program's return value and from there in a map that userspace can read.

Before 0ee21f37b2cb the AAPCS64 placement never split an argument
(arm64_arg_abi has no split_at_boundary), so the per-argument classification
was safe.  The identity mapping, whose comment says "no split refused",
breaks that assumption.  x86 doesn't hit this because its
save_args()/restore_regs() classify each slot.

The commit message describes callee_stack() as covering arm64, but only the
x86-64 failure output is shown, and no commit in the series changes
arch/arm64/net/bpf_jit_comp.c.  Was this run on arm64?

Either the arm64 trampoline needs per-slot handling for models with
bpf_tgt_prog set, or the callee_stack case needs to be gated off there.

> +		ASSERT_EQ(tracer->bss->stack_i, 10, "stack:i");
> +		ASSERT_EQ(tgt->bss->ret_stack, 385, "stack ret");
> +		ASSERT_EQ(tracer->bss->stack_ret, 385, "stack fexit ret");
> +	}

[ ... ]


---
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/37897160282

Re: [PATCH bpf-next 1/5] selftests/bpf: Test by-value __int128 kfunc arguments at an odd slot

[Yonghong Song]

On 10/9/26 9:48 AM, bot+bpf-ci@kernel.org wrote:
>> selftests/bpf: Test by-value __int128 kfunc arguments at an odd slot
> TL;DR: On arm64 with an older pahole, the two new kfuncs lack BTF, so
> bpf_testmod.ko may fail to build or load, losing all bpf_testmod selftests,
> not just the new ones.
>
>> On arm64 the first two kfuncs get BTF only with the pahole patch
>> "dwarf_loader: Skip the argument register the arm64 ABI leaves as a hole"
>> [2], in pahole master after v1.32. An older pahole finds the argument
>> after the __int128 in an unexpected register, x4 rather than x3, and
>> leaves the function out of BTF, so the tests fail to load there.
>> diff --git a/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c b/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c
>> --- a/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c
>> +++ b/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c
> [ ... ]
>
>> @@ -1763,6 +1786,9 @@ 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_i128_arg_odd)
>> +BTF_ID_FLAGS(func, bpf_kfunc_call_test_i128_arg_odd_many)
>> +BTF_ID_FLAGS(func, bpf_kfunc_call_test_i128_arg_stack_hole)
>>   BTF_ID_FLAGS(func, bpf_kfunc_call_test_i128_arg_pad)
> Is the failure on arm64 with an older pahole really limited to the two new
> tests?

Yes, we use the latest pahole in github. Note that latest pahole is
also available to CI.

>
> Both bpf_kfunc_call_test_i128_arg_odd and
> bpf_kfunc_call_test_i128_arg_odd_many are added to
> bpf_testmod_check_kfunc_ids, the module's main kfunc set.
>
> With no BTF FUNC for them, resolve_btfids only warns ("no BTF func for
> kfunc ..." in collect_kfuncs() and "unresolved symbol ..." in id_patch())
> and leaves their set8 ids at 0.  With CONFIG_WERROR=y, scripts/Makefile.btf
> passes --fatal_warnings, so bpf_testmod.ko would fail to build.
>
> Otherwise, at module load bpf_testmod_init() calls
> register_btf_kfunc_id_set(BPF_PROG_TYPE_SCHED_CLS, &bpf_testmod_kfunc_set),
> and __register_btf_kfunc_id_set() checks every entry:
>
>      for (i = 0; i < kset->set->cnt; i++) {
>          ret = btf_check_kfunc_protos(btf, btf_relocate_id(btf, kset->set->pairs[i].id), ...);
>          if (ret)
>              goto err_out;
>      }
>
> btf_check_kfunc_protos() does:
>
>      func = btf_type_by_id(btf, func_id);
>      if (!func || !btf_type_is_func(func))
>          return -EINVAL;
>
> Id 0 resolves to btf_void in the base BTF (btf_relocate_id() maps 0 to 0),
> so this returns -EINVAL.
>
> That would reject the whole set, bpf_testmod_init() would return the error,
> and bpf_testmod.ko would not load.  test_progs then prints "Selftests
> relying on bpf_testmod.ko will be skipped", so every bpf_testmod-dependent
> selftest on arm64 would be lost, not only the two new ones.  The same
> configuration worked before this patch.
>
> Could the two arm64-hole kfuncs go into a separate kfunc set whose
> registration failure is tolerated?  Alternatively, should the commit message
> say that bpf_testmod.ko as a whole needs the newer pahole on arm64, rather
> than "the tests fail to load there"?

There is no need to test with old pahole. Rather we should use latest pahole.

>
>
> ---
> 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/37897160282

Re: [PATCH bpf-next 2/5] bpf: Keep the BPF calling convention for a trampoline on a BPF function

[Yonghong Song]

On 10/9/26 9:26 AM, bot+bpf-ci@kernel.org wrote:
>> bpf: Keep the BPF calling convention for a trampoline on a BPF function
> TL;DR: On arm64, the identity slot mapping for BPF targets may split a 16-byte
> argument across x7 and the stack; fexit/fentry could then read x8 or
> uninitialised stack, corrupting the traced function's values.
>
>> diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
>> --- a/kernel/bpf/core.c
>> +++ b/kernel/bpf/core.c
> [ ... ]
>
>> @@ -3342,6 +3342,18 @@ u32 bpf_jit_place_args(const struct bpf_jit_arg_abi *abi,
>>   	u32 i, k, nslots, slot = 0, nregs_used = 0, stack_off = 0;
>>   	bool on_stack = false;
>>
>> +	/*
>> +	 * A BPF function is called by BPF code, which passes slot N where
>> +	 * the kernel convention passes its Nth eightbyte, whatever the
>> +	 * arguments: no hole, no split refused and no register backfilled.
>> +	 */
>> +	if (fm->bpf_tgt_prog) {
>> +		for (i = 0; i < fm->nr_args; i++)
>> +			for (k = btf_func_model_arg_slots(fm, i); k > 0; k--, slot++)
>> +				pos_of_slot[slot] = slot;
>> +		return slot;
>> +	}
>> +
> Can this identity mapping split one 16-byte argument across the last
> register and the stack on arm64, which is the only trampoline calling
> bpf_jit_place_args() at this commit?
>
> Before this patch, arm64_arg_abi has no split_at_boundary, so every slot of
> an argument landed either in x0-x7 or on the stack.  calc_arg_aux() relies
> on that and only checks the first slot of each argument:
>
>      for (i = 0, slot = 0; i < m->nr_args; i++) {
>          slots = (m->arg_size[i] + 7) / 8;
>          if (a64_arg_on_stack(a->pos_of_slot[slot])) /* passed through register ? */
>              break;
>          slot += slots;
>      }
>
> save_args() then uses each pos_of_slot value of such an "in register"
> argument as a register number:
>
>      reg = a->pos_of_slot[slot++];
>      if (for_call_origin) {
>          emit(A64_LDR64I(reg, A64_SP, bargs_off), ctx);
>      ...
>      } else {
>          emit(A64_STR64I(reg, A64_SP, bargs_off), ctx);
>      }
>
> With the identity mapping, a 16-byte argument at BPF slot 7 gets
> pos_of_slot = {7, 8}.  This looks reachable: a static subprog can have up
> to 12 argument slots (btf_check_arg_slots(), and
> bpf_jit_supports_stack_args() is true on arm64), and btf_prepare_func_args()
> accepts scalar-only structs up to 16 bytes.  The selftest added later in
> this series (e5dd7e2f7244) has such a function:
>
>      static __noinline __u64 callee_stack(__u64 a, __u64 b, __u64 c, __u64 d,
>                                           __u64 e, __u64 f, __u64 g,
>                                           struct int128_arg h, __u64 i)
>
> The arm64 JIT passes h.lo in x7 (stack_arg_reg[2]) and h.hi at [SP+0]
> (emit_stack_arg_store(), idx 3).  calc_arg_aux() then returns
> args_in_regs = 8, regs_for_args = 9, bstack_for_args = 8 and
> ostack_for_args = 16, so:
>
> - fentry: save_args() stores A64_R(8) into ctx[8].  On arm64, x8 is
>    BPF_REG_0 of the calling BPF code (bpf2a64[BPF_REG_0] = A64_R(8)), not
>    h.hi, which is still at FP + 32 + 0.
>
> - fexit (BPF_TRAMP_F_CALL_ORIG): save_args(..., true, ...) loads the saved
>    value into x8 and never writes oargs_off + 0.  The original function then
>    reads h.hi from uninitialised trampoline stack.  If the split argument is
>    the last one, bstack_for_args and ostack_for_args are both 0, and the
>    function reads [SP+0] of the trampoline frame, which is the
>    bpf_tramp_run_ctx.
>
> Attaching fexit would then change the values the traced BPF function
> computes with, and what it returns.
>
> The new comment says the BPF convention has "no split refused", and the x86
> follow-up f5df67b558e5 ("bpf, x86: Place trampoline arguments by the x86-64
> calling convention") handles a split per slot for x86.  But no commit in the
> range updates arch/arm64/net/bpf_jit_comp.c.  The int128_tgt_prog selftest
> in e5dd7e2f7244 also runs on __aarch64__, where it would expect ctx[8] == 9
> but read x8.
>
> Should calc_arg_aux(), save_args() and restore_args() on arm64 classify and
> read each slot by its own position (register vs. FP + 32 +
> a64_arg_stack_off()), the way the x86 follow-up does, so that a split
> 16-byte argument works?

Indeed, I missed arm64 case. Will fix it in the next revision.

>
>
> ---
> 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/37897160282

Re: [PATCH bpf-next 3/5] bpf, x86: Place trampoline arguments by the x86-64 calling convention

[Yonghong Song]

On 10/9/26 9:08 AM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [High] bpf, x86: stack arguments offset mismatch and misalignment with BPF_TRAMP_F_TAIL_CALL_CTX
> - [Medium] bpf: info leak of uninitialized kernel stack memory due to missing garbage cleanup
> --
>
> commit ffb54c9ff7ace6fffcfc471ab2c2927e34131e44
> Author: Yonghong Song <yonghong.song@linux.dev>
>
> bpf, x86: Place trampoline arguments by the x86-64 calling convention
>
> This commit updates the BPF trampoline to place on-stack arguments
> according to the x86-64 calling convention. It introduces a backfill
> mechanism using bpf_jit_place_args() to handle arguments like __int128
> that may span stack slots and create alignment holes.
>
>> diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
>> index 083fcd6cf15b7..0fc7715e569b5 100644
>> --- a/arch/x86/net/bpf_jit_comp.c
>> +++ b/arch/x86/net/bpf_jit_comp.c
> [ ... ]
>> @@ -3472,8 +3479,35 @@ static void save_args(const struct btf_func_model *m, u8 **prog,
>>   		 *
>>   		 * the arg1-5,arg7 will be passed by regs, and arg6 will
>>   		 * by stack.
>> +		 *
>> +		 * An argument aligned to 16 bytes, an __int128 or a struct
>> +		 * holding one, also starts at an even stack slot, which can
>> +		 * leave a hole before it. bpf_jit_place_args() worked out
>> +		 * where each eightbyte is.
>>   		 */
>> -		if (nr_regs + arg_regs > 6) {
>> +		for (j = 0; j < arg_regs; j++, slot++, bargs_off -= 8) {
>> +			u8 pos = pos_of_slot[slot];
>> +			u32 src;
>> +
>> +			if (!x86_arg_on_stack(pos)) {
> [ ... ]
>> +			/* copy function arguments from origin stack frame
>> +			 * into current stack frame.
> [ ... ]
>> +			 *
>> +			 * The area for the origin call mirrors the incoming
>> +			 * one, hole and all; only the bpf program takes the
>> +			 * arguments packed.
>>   			 */
>> -			for (j = 0; j < arg_regs; j++) {
>> -				emit_ldx(prog, BPF_DW, BPF_REG_0, BPF_REG_FP,
>> -					 nr_stack_slots * 8 + stack_args_off);
>> -				if (arena_arg)
>> -					emit_arena_arg_conv(prog, BPF_REG_0, nullable,
>> -							    (u32)arena_base);
>> -				emit_stx(prog, BPF_DW, BPF_REG_FP, BPF_REG_0,
>> -					 -stack_size);
>> -
>> -				if (!nr_stack_slots)
>> -					first_off = stack_size;
>> -				stack_size -= 8;
>> -				nr_stack_slots++;
>> -			}
>> +			off = x86_arg_stack_off(pos);
>> +			doff = for_call_origin ? stack_size - off : bargs_off;
>> +			emit_ldx(prog, BPF_DW, BPF_REG_0, BPF_REG_FP,
>> +				 stack_args_off + off);
>> +			if (arena_arg)
>> +				emit_arena_arg_conv(prog, BPF_REG_0, nullable,
>> +						    (u32)arena_base);
>> +			emit_stx(prog, BPF_DW, BPF_REG_FP, BPF_REG_0, -doff);
>> +
>> +			if (!nr_stack_slots)
>> +				first_off = doff;
>> +			nr_stack_slots++;
>>   		}
>>   	}
>>   
>>   	clean_stack_garbage(m, prog, nr_stack_slots, first_off);
>>   }
> [Severity: Medium]
> Does this code leak uninitialized kernel stack memory?
>
> The x86-64 ABI allocates 8-byte stack slots for arguments, but compilers
> frequently use 32-bit instructions to place 32-bit arguments, leaving the upper
> 4 bytes of the slot uninitialized.
>
> Since clean_stack_garbage() explicitly returns early when nr_stack_slots != 1,
> copying the full 64 bits with emit_ldx(..., BPF_DW, ...) captures this
> uninitialized kernel memory into the BPF context. Can this leak kernel stack
> memory to privileged BPF users if there are multiple stack arguments?

We probably will be okay since for bpf program, the return value will be
narrowed based on return type.

>
> [ ... ]
>> @@ -4011,8 +4014,8 @@ static int __arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *rw_im
>>   	}
>>   
>>   	if (flags & BPF_TRAMP_F_CALL_ORIG) {
>> -		restore_regs(m, &prog, regs_off);
>> -		save_args(m, &prog, arg_stack_off, true, flags, 0);
>> +		restore_regs(&prog, pos_of_slot, nr_regs, regs_off);
>> +		save_args(m, &prog, pos_of_slot, arg_stack_off, true, flags, 0);
>>   
>>   		if (flags & BPF_TRAMP_F_TAIL_CALL_CTX) {
>>   			/* Before calling the original function, load the
> [Severity: High]
> Are stack arguments misaligned here if BPF_TRAMP_F_TAIL_CALL_CTX is active?
>
> When __arch_prepare_bpf_trampoline() executes, it checks for
> BPF_TRAMP_F_TAIL_CALL_CTX and emits a push rax instruction (pushing 8 bytes)
> after sub rsp, stack_size, which permanently shifts RSP down by 8 bytes.
>
> However, the updated save_args() calculates the stack argument offsets based on
> the original stack_size (doff = stack_size - off) without accounting for this
> extra 8-byte shift.
>
> Because the push rax is unpopped before call orig_call, the original function
> will read its first stack argument from [RSP + 8]. Due to the 8-byte shift,
> this points to the tail call counter pointer instead of the actual first stack
> argument, corrupting all on-stack arguments.
>
> Additionally, doesn't this 8-byte shift break the required 16-byte stack
> alignment on function entry for x86-64?

This patch just allows 16-byte aligned parameters with possible holes. The
rest of change should be the same as the original code. Here you mentioned
a 'push rax' before calling orig_call. These should be okay, 16-byte aligned
parameters may have holes but it should not damage the 'push rax' or calling
orig_call. I will see whether we can add some tests for this.

Re: [PATCH bpf-next 4/5] selftests/bpf: Test trampoline args around a 16-byte aligned stack arg

[Yonghong Song]

On 10/9/26 8:51 AM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [Medium] bpf: endianness mismatch when reading __int128 in selftests on big-endian
> - [Medium] selftests/bpf: data race on bpf_testmod_test_struct_arg_result
> --
>
> commit 44831c6b1a0172e1d0713d17a8e105c4b30d6d64
> Author: Yonghong Song <yonghong.song@linux.dev>
>
> selftests/bpf: Test trampoline args around a 16-byte aligned stack arg
>
> This commit introduces two new bpf_testmod functions to test trampoline
> argument handling for 16-byte aligned stack arguments, specifically targeting
> __int128 structures. It also adds fentry and fexit programs to verify that
> arguments are correctly packed in the context array according to BPF
> conventions on x86-64 and arm64.
>
>> diff --git a/tools/testing/selftests/bpf/progs/tracing_struct_int128.c b/tools/testing/selftests/bpf/progs/tracing_struct_int128.c
>> index 4638dfec1f38e..5520ee500c01d 100644
>> --- a/tools/testing/selftests/bpf/progs/tracing_struct_int128.c
>> +++ b/tools/testing/selftests/bpf/progs/tracing_struct_int128.c
>> @@ -15,4 +15,49 @@ int test_int128_arg_fexit(unsigned long long *ctx)
>>   	return 0;
>>   }
>>   
>> +/*
>> + * The arguments come packed in the BPF convention whatever holes the
>> + * kernel convention left between them: an eightbyte each, and two for the
>> + * struct holding an __int128.
>> + */
>> +long s_g, s_h_lo, s_h_hi, s_i, s_ret;
>> +
>> +SEC("fentry/bpf_testmod_test_int128_stack")
>> +int test_int128_stack_fentry(unsigned long long *ctx)
>> +{
>> +	s_g = ctx[6];
>> +	s_h_lo = ctx[7];
>> +	s_h_hi = ctx[8];
> [Severity: Medium]
> Will this little-endian assumption cause the selftest to fail on big-endian
> systems? On big-endian arm64 (aarch64_be), the high 64 bits of the 16-byte
> struct would be at the lower memory address, so reading ctx[7] as the low
> 64 bits and ctx[8] as the high 64 bits might be reversed.

Okay, I will use __BYTE_ORDER__ to guard. The same for below.

>
>> +	s_i = ctx[9];
>> +	return 0;
>> +}
>> +
> [ ... ]
>
>> +long b_f_a, b_f_b, b_g, b_h, b_i_lo, b_i_hi, b_ret;
>> +
>> +SEC("fentry/bpf_testmod_test_int128_backfill")
>> +int test_int128_backfill_fentry(unsigned long long *ctx)
>> +{
>> +	b_f_a = ctx[5];
>> +	b_f_b = ctx[6];
>> +	b_g = ctx[7];
>> +	b_h = ctx[8];
>> +	b_i_lo = ctx[9];
>> +	b_i_hi = ctx[10];
> [Severity: Medium]
> Does this have the same endianness mismatch on big-endian architectures?
> Extracting the 16-byte struct here from ctx[9] and ctx[10] also seems to
> assume a little-endian memory layout.
>
>> +	return 0;
>> +}
>> diff --git a/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c b/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c
>> index 54a5fc39e0e27..23a2c88d3e7c1 100644
>> --- a/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c
>> +++ b/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c
> [ ... ]
>
>> +noinline long
>> +bpf_testmod_test_int128_stack(u64 a, u64 b, u64 c, u64 d, u64 e, u64 f,
>> +			      u64 g, struct bpf_testmod_struct_int128 h, u64 i)
>> +{
>> +	bpf_testmod_test_struct_arg_result = a + b * 2 + c * 3 + d * 4 + e * 5 +
>> +		f * 6 + g * 7 + (u64)h.a * 8 +
>> +		(u64)((unsigned __int128)h.a >> 64) * 9 + i * 10;
>> +	return bpf_testmod_test_struct_arg_result;
>> +}
> [Severity: Medium]
> Can concurrent execution of tests (like test_progs -j) lead to a data race on
> this global variable? If one test thread computes the result in
> bpf_testmod_test_int128_stack() and another thread overwrites
> bpf_testmod_test_struct_arg_result in a different function before the first
> returns, it could cause the BPF fexit hook to read an incorrect return value
> into s_ret, leading to flaky ASSERT_EQ failures.

I will double check this. One thing we could do is to have the *same* inputs.

>
>> +
>> +/*
>> + * x86-64 has no register left for f after e, so f goes to the stack and g
>> + * takes R9 back; h goes to the third stack slot and i, aligned to 16
>> + * bytes, to the fifth and sixth. arm64 has registers up to g, puts h in
>> + * the first stack slot and i in the third and fourth.
>> + */
>> +noinline long
>> +bpf_testmod_test_int128_backfill(u64 a, u64 b, u64 c, u64 d, u64 e,
>> +				 struct bpf_testmod_struct_arg_2 f, u64 g, u64 h,
>> +				 struct bpf_testmod_struct_int128 i)
>> +{
>> +	bpf_testmod_test_struct_arg_result = a + b * 2 + c * 3 + d * 4 + e * 5 +
>> +		f.a * 6 + f.b * 7 + g * 8 + h * 9 + (u64)i.a * 10 +
>> +		(u64)((unsigned __int128)i.a >> 64) * 11;
>> +	return bpf_testmod_test_struct_arg_result;
> [Severity: Medium]
> Since this function also writes to the unprotected global variable
> bpf_testmod_test_struct_arg_result and immediately returns it, could this
> contribute to the same data race when multiple test processes run
> trigger_module_test_read() concurrently?
>
>> +}
>>   #endif

Re: [PATCH bpf-next 5/5] selftests/bpf: Test trampoline args of a BPF function at an odd slot

[Yonghong Song]

On 10/9/26 9:48 AM, bot+bpf-ci@kernel.org wrote:
>> selftests/bpf: Test trampoline args of a BPF function at an odd slot
> TL;DR:
> - On arm64, the callee_stack case may read an uninitialized stack value for
>    h.hi, so its assertions may fail
> - Silent test__skip() without a message (nit, matches existing idiom)
>
>> diff --git a/tools/testing/selftests/bpf/prog_tests/tracing_struct.c b/tools/testing/selftests/bpf/prog_tests/tracing_struct.c
>> --- a/tools/testing/selftests/bpf/prog_tests/tracing_struct.c
>> +++ b/tools/testing/selftests/bpf/prog_tests/tracing_struct.c
> [ ... ]
>
>> @@ -162,6 +164,76 @@ static void test_int128_args(void)
>>   #endif
>>   }
>>
>> +static void test_int128_tgt_prog(void)
>> +{
>> +#if defined(__x86_64__) || defined(__aarch64__)
>> +	struct tracing_struct_int128_tracer *tracer = NULL;
>> +	struct tracing_struct_int128_tgt *tgt;
>> +	LIBBPF_OPTS(bpf_test_run_opts, topts);
>> +	int err, fd;
>> +
>> +	tgt = tracing_struct_int128_tgt__open_and_load();
>> +	if (!ASSERT_OK_PTR(tgt, "tgt__open_and_load"))
>> +		return;
>> +	if (!tgt->rodata->has_struct_arg) {
>> +		test__skip();
>> +		goto out;
> A subsystem pattern flags this as potentially concerning, though this isn't
> a bug: test__skip() is called here without any message, and the #else branch
> for other architectures does the same.  When the target object was built
> without by-value struct argument support (a GCC BPF build), the run shows a
> bare SKIP with nothing explaining what was missing.
>
> The pattern says a test that skips silently with no message should be
> reported.  test__skip() takes no message argument, and test_int128_args()
> and prog_tests/stack_arg.c use the same bare test__skip() call, so this may
> simply match the existing idiom.
>
> Would a printf() of the reason before test__skip() be worth adding here?

I will keep the current way.

>
>> +	}
>> +	fd = bpf_program__fd(tgt->progs.run);
> [ ... ]
>
>> +	if (tgt->rodata->has_stack_arg) {
>> +		ASSERT_EQ(tracer->bss->stack_g, 7, "stack:g");
>> +		ASSERT_EQ(tracer->bss->stack_lo, 8, "stack:h.lo");
>> +		ASSERT_EQ(tracer->bss->stack_hi, 9, "stack:h.hi");
> Does the callee_stack half of this subtest pass on arm64?
>
> In the BPF convention, callee_stack(a, b, c, d, e, f, g, struct int128_arg h,
> i) uses argument slots 0..6 for a..g, 7..8 for h and 9 for i.  The arm64 JIT
> passes BPF slots 5..7 in x5..x7 and slot 8 onward on the stack, so h.lo is in
> x7 and h.hi is at [SP + 0].
>
> For a BPF target, commit 0ee21f37b2cb ("bpf: Keep the BPF calling convention
> for a trampoline on a BPF function") makes bpf_jit_place_args() return the
> identity mapping:
>
>      if (fm->bpf_tgt_prog) {
>          for (i = 0; i < fm->nr_args; i++)
>              for (k = btf_func_model_arg_slots(fm, i); k > 0; k--, slot++)
>                  pos_of_slot[slot] = slot;
>
> The arm64 code decides whether an argument is in registers or on the stack
> from its first slot only, in calc_arg_aux():
>
>      if (a64_arg_on_stack(a->pos_of_slot[slot])) /* passed through register ? */
>          break;
>      slot += slots;
>
> h starts at pos 7, so it is counted as a register argument (args_in_regs = 8,
> regs_for_args = 9).  Then save_args() handles h's second slot with:
>
>      reg = a->pos_of_slot[slot++];
>      ...
>      emit(A64_STR64I(reg, A64_SP, bargs_off), ctx);
>
> with reg = 8.  That stores x8 (BPF R0 in bpf2a64[]) into ctx[8] instead of
> reading h.hi from the stack.  On the call-origin path the same loop reloads
> x8, and the stack loop writes only i, at oargs_off + 8.  The outgoing slot 0
> is never written, so callee_stack() would read h.hi from uninitialized
> trampoline stack.
>
> With a clang that defines __BPF_FEATURE_STACK_ARGUMENT, should the
> "stack:h.hi", "stack ret" and "stack fexit ret" assertions be expected to
> fail on arm64?  A leftover kernel stack value would also end up in the traced
> program's return value and from there in a map that userspace can read.
>
> Before 0ee21f37b2cb the AAPCS64 placement never split an argument
> (arm64_arg_abi has no split_at_boundary), so the per-argument classification
> was safe.  The identity mapping, whose comment says "no split refused",
> breaks that assumption.  x86 doesn't hit this because its
> save_args()/restore_regs() classify each slot.
>
> The commit message describes callee_stack() as covering arm64, but only the
> x86-64 failure output is shown, and no commit in the series changes
> arch/arm64/net/bpf_jit_comp.c.  Was this run on arm64?
>
> Either the arm64 trampoline needs per-slot handling for models with
> bpf_tgt_prog set, or the callee_stack case needs to be gated off there.

This has the same issue at commit 2. Will fix.

>
>> +		ASSERT_EQ(tracer->bss->stack_i, 10, "stack:i");
>> +		ASSERT_EQ(tgt->bss->ret_stack, 385, "stack ret");
>> +		ASSERT_EQ(tracer->bss->stack_ret, 385, "stack fexit ret");
>> +	}
> [ ... ]
>
>
> ---
> 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/37897160282

Re: [PATCH bpf-next 2/5] bpf: Keep the BPF calling convention for a trampoline on a BPF function

[Alexei Starovoitov]

On Thu, Oct 08, 2026 at 11:42 PM Yonghong Song <yonghong.song@linux.dev> wrote:
> Mark the model of a target program's function as bpf_tgt_prog, and have
> bpf_jit_place_args() leave every slot of such a model where the BPF
> convention put it. That is where both JITs pass it in a call from one
> BPF function to another: slot N where the kernel convention passes its
> Nth eightbyte, in a register or a stack slot. The x86-64 trampoline does
> not use bpf_jit_place_args() yet; the next patch makes it, and this
> keeps it right for a BPF target.

can we drop the flag and for tgt_prog rewrite the model in
bpf_check_attach_target() as one u64 arg per argument slot ?
Every convention places N u64 args in slot order,
so no arch has to know about BPF targets.

The flag fixes bpf_jit_place_args() only.
calc_arg_aux() on arm64 picks reg vs stack per argument by looking
at its first slot. In callee_stack() from patch 5 'h' is in slots 7 and 8.
pos_of_slot[7] is x7, so the whole 'h' is counted in args_in_regs and
save_args() emits 'str x8' for the second half. x8 is BPF R0.
ctx[8] is garbage. For fexit it does 'ldr x8' and the first outgoing
stack slot is never written, so callee_stack() runs with garbage
in the upper half of 'h'. The test should fail on arm64.
CI uses clang 21 without stack args and won't catch it.

s390 trampoline doesn't use bpf_jit_place_args().
s390 ABI passes 16 byte arg by reference, so it does:
  mvc bpf_arg_off(16,%r15),0(%arg)
For callee_odd() that is a load from address 8.
It also saves and restores nr_args == 3 registers,
while callee_odd() takes 4.

With the flat model arm64, s390 and the r9/stack split on x86
are fixed without touching arch code.

> @@ -21620,6 +21620,7 @@ int bpf_check_attach_target(struct bpf_verifier_log *log,
>  		ret = btf_distill_func_proto(log, btf, t, tname, &tgt_info->fmodel);
>  		if (ret < 0)
>  			return ret;
> +		tgt_info->fmodel.bpf_tgt_prog = !!tgt_prog;

There is one more issue with BPF targets. Few lines above:
	if (tgt_prog && conservative)
		t = NULL;
and the model is 5 u64.
A static function with 6+ args where one of them is a plain pointer
is 'unreliable', since btf_prepare_func_args() returns -EINVAL,
but it still takes stack args.
The trampoline saves and restores 5 registers. After fentry prog
r9 on x86 and x5-x7 on arm64 have whatever __bpf_prog_exit() left there.
With fexit the stack slots are not copied either.
The function continues with garbage in the args that the verifier
checked as pointers, I think.
Reject such attach ?

Pls tell your agent to test on arm64 too.

pw-bot: cr

Re: [PATCH bpf-next 2/5] bpf: Keep the BPF calling convention for a trampoline on a BPF function

[Yonghong Song]

On 10/10/26 5:00 PM, Alexei Starovoitov wrote:
> On Thu, Oct 08, 2026 at 11:42 PM Yonghong Song <yonghong.song@linux.dev> wrote:
>> Mark the model of a target program's function as bpf_tgt_prog, and have
>> bpf_jit_place_args() leave every slot of such a model where the BPF
>> convention put it. That is where both JITs pass it in a call from one
>> BPF function to another: slot N where the kernel convention passes its
>> Nth eightbyte, in a register or a stack slot. The x86-64 trampoline does
>> not use bpf_jit_place_args() yet; the next patch makes it, and this
>> keeps it right for a BPF target.
> can we drop the flag and for tgt_prog rewrite the model in
> bpf_check_attach_target() as one u64 arg per argument slot ?
> Every convention places N u64 args in slot order,
> so no arch has to know about BPF targets.

Good point. Will do this with u64 arg per argument slot.

>
> The flag fixes bpf_jit_place_args() only.
> calc_arg_aux() on arm64 picks reg vs stack per argument by looking
> at its first slot. In callee_stack() from patch 5 'h' is in slots 7 and 8.
> pos_of_slot[7] is x7, so the whole 'h' is counted in args_in_regs and
> save_args() emits 'str x8' for the second half. x8 is BPF R0.
> ctx[8] is garbage. For fexit it does 'ldr x8' and the first outgoing
> stack slot is never written, so callee_stack() runs with garbage
> in the upper half of 'h'. The test should fail on arm64.

Indeed, this is an issue. I should test with llvm23 for arm64 as well.

> CI uses clang 21 without stack args and won't catch it.
>
> s390 trampoline doesn't use bpf_jit_place_args().
> s390 ABI passes 16 byte arg by reference, so it does:
>    mvc bpf_arg_off(16,%r15),0(%arg)
> For callee_odd() that is a load from address 8.
> It also saves and restores nr_args == 3 registers,
> while callee_odd() takes 4.
>
> With the flat model arm64, s390 and the r9/stack split on x86
> are fixed without touching arch code.
>
>> @@ -21620,6 +21620,7 @@ int bpf_check_attach_target(struct bpf_verifier_log *log,
>>   		ret = btf_distill_func_proto(log, btf, t, tname, &tgt_info->fmodel);
>>   		if (ret < 0)
>>   			return ret;
>> +		tgt_info->fmodel.bpf_tgt_prog = !!tgt_prog;
> There is one more issue with BPF targets. Few lines above:
> 	if (tgt_prog && conservative)
> 		t = NULL;
> and the model is 5 u64.
> A static function with 6+ args where one of them is a plain pointer
> is 'unreliable', since btf_prepare_func_args() returns -EINVAL,
> but it still takes stack args.
> The trampoline saves and restores 5 registers. After fentry prog
> r9 on x86 and x5-x7 on arm64 have whatever __bpf_prog_exit() left there.
> With fexit the stack slots are not copied either.
> The function continues with garbage in the args that the verifier
> checked as pointers, I think.
> Reject such attach ?

I agree that rejection is the right way to go.

>
> Pls tell your agent to test on arm64 too.

Yes, will do.

>
> pw-bot: cr