[PATCH bpf-next v2 00/11] bpf: Follow up by-value struct and __int128 argument support

[PATCH bpf-next v2 00/11] 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, and a few
problems found on the way:

  - 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 argument
    slot N where the kernel convention passes its Nth u64 argument, but
    the trampolines placed the arguments by their types: arm64 left a
    register empty before a 16-byte argument, and x86-64 would not split
    one between r9 and the stack. Such a function is now described to
    its trampoline as one u64 per slot.

  - The x86-64 trampoline on a function of a program that can tail call
    pushed the tail call counter pointer between the stack arguments and
    the return address.

  - A static function whose BTF is unreliable could still take stack
    arguments, while its trampoline saved and restored only five
    registers. Attaching to such a function is now refused.

  - The x86-64 JIT passed a kfunc's 16-byte aligned stack argument at an
    address that was only 8-byte aligned, while the callee may rely on
    the alignment.

  - 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 keeps the x86-64 trampoline's tail call counter pointer clear of
the stack arguments, and patch 2 tests it. Patch 3 adds the odd-slot
kfunc tests. Patch 4 describes a BPF function to its trampoline as one
u64 per slot, and patch 5 places the x86-64 trampoline arguments by the
calling convention. Patches 6 and 7 test trampolines on kernel functions
and on BPF functions with an argument after a 16-byte aligned one. Patch
8 refuses a trampoline on an unreliable static function with stack
arguments, and patch 9 tests it, along with a BPF function whose 16-byte
argument x86-64 splits between r9 and the stack. Patch 10 passes a
kfunc's 16-byte aligned stack argument 16-byte aligned on x86-64, and
patch 11 tests it.

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

Changelogs:
  v1 -> v2:
    - v1: https://lore.kernel.org/bpf/20261009064206.1238047-1-yonghong.song@linux.dev/
    - Replace the bpf_tgt_prog flag of v1 patch 2, which only
      bpf_jit_place_args() looked at, with a model of one u64 per
      argument slot for a BPF function, built in
      bpf_check_attach_target(). This also fixes the arm64 trampoline,
      which took a 16-byte argument split between x7 and the stack as
      wholly in registers, without any arch change. (patch 4)
    - Refuse a trampoline on a static function whose BTF is unreliable
      but which takes stack arguments. (patches 8 and 9)
    - New: keep the x86-64 trampoline's tail call counter pointer in a
      slot of its frame instead of pushing it between the stack
      arguments and the return address. (patches 1 and 2)
    - New: pass a kfunc's 16-byte aligned stack argument 16-byte aligned
      in the x86-64 JIT, with bpf_jit_stack_slots_to_align() and
      bpf_jit_plan_slot_moves() in kernel/bpf/core.c. (patches 10 and
      11)
    - Reword the x86-64 placement patch and the BPF function test for
      the new model. (patches 5 and 7)

Yonghong Song (11):
  bpf, x86: Pass a trampoline's stack arguments by the calling
    convention
  selftests/bpf: Test trampoline stack args in a program that can tail
    call
  selftests/bpf: Test by-value __int128 kfunc arguments at an odd slot
  bpf: Describe a BPF function to its trampoline as one u64 per slot
  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
  bpf: Refuse a trampoline on an unreliable static function with stack
    args
  selftests/bpf: Test trampolines on more BPF functions with stack args
  bpf, x86: Pass a kfunc's 16-byte aligned stack argument 16-byte
    aligned
  selftests/bpf: Test that a kfunc gets a 16-byte aligned __int128
    aligned

 arch/x86/net/bpf_jit_comp.c                   | 274 +++++++++++-------
 include/linux/filter.h                        |   5 +
 kernel/bpf/core.c                             |  50 +++-
 kernel/bpf/verifier.c                         |  66 ++++-
 .../selftests/bpf/prog_tests/tracing_struct.c | 217 +++++++++++++-
 .../selftests/bpf/progs/aggregate_arg_kfunc.c | 113 ++++++++
 .../progs/tracing_stack_arg_tailcall_tgt.c    |  59 ++++
 .../progs/tracing_stack_arg_tailcall_tracer.c |  23 ++
 .../bpf/progs/tracing_struct_int128.c         |  45 +++
 .../bpf/progs/tracing_struct_int128_tgt.c     |  95 ++++++
 .../bpf/progs/tracing_struct_int128_tracer.c  |  70 +++++
 .../selftests/bpf/test_kmods/bpf_testmod.c    |  84 ++++++
 .../bpf/test_kmods/bpf_testmod_kfunc.h        |  15 +
 13 files changed, 999 insertions(+), 117 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/progs/tracing_stack_arg_tailcall_tgt.c
 create mode 100644 tools/testing/selftests/bpf/progs/tracing_stack_arg_tailcall_tracer.c
 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 v2 01/11] bpf, x86: Pass a trampoline's stack arguments by the calling convention

[Yonghong Song]

A trampoline on a function of a program that can tail call keeps the
tail call counter pointer by pushing rax after allocating its frame.
When it calls the original function, for fexit, fmod_ret or fsession,
the push sits between the stack arguments and the return address, which
the x86-64 calling convention does not allow. For example, in

	run() -> call_stack() -> callee_stack(a, b, c, d, e, f, g, h)
	run() -> subprog_tail() -> bpf_tail_call()

callee_stack() is not on the path to the tail call, so the verifier
allows its stack arguments g and h. fexit on it builds, with T the
trampoline's rbp:

	T-120  h                  save_args()
	T-128  g                  save_args(), RSP after sub rsp, 128
	T-136  tail_call_cnt_ptr  push rax, RSP at the call
	T-144  return address     call orig_call

and callee_stack() finds the counter pointer where g belongs, at 8(%rsp)
on entry, and g where h belongs.

Keep the pointer in a slot of the frame instead, so that the stack
arguments sit right above the return address again:

	T-120  tail_call_cnt_ptr  mov [rbp - 120], rax
	T-128  (pad to 16)
	T-136  h                  save_args()
	T-144  g                  save_args(), RSP at the call
	T-152  return address     call orig_call

Fixes: 324c3ca6eed6 ("bpf,x86: Implement JIT support for stack arguments")
Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
---
 arch/x86/net/bpf_jit_comp.c | 22 +++++++++++++++-------
 1 file changed, 15 insertions(+), 7 deletions(-)

diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
index 083fcd6cf15b..3982507d54ca 100644
--- a/arch/x86/net/bpf_jit_comp.c
+++ b/arch/x86/net/bpf_jit_comp.c
@@ -3743,9 +3743,9 @@ static int invoke_bpf_mod_ret(const struct btf_func_model *m, u8 **pprog,
 	return 0;
 }
 
-/* mov rax, qword ptr [rbp - rounded_stack_depth - 8] */
-#define LOAD_TRAMP_TAIL_CALL_CNT_PTR(stack)	\
-	__LOAD_TCC_PTR(-round_up(stack, 8) - 8)
+/* mov rax, qword ptr [rbp - off] */
+#define LOAD_TRAMP_TAIL_CALL_CNT_PTR(off)	\
+	__LOAD_TCC_PTR(-(off))
 
 /* Example:
  * __be16 eth_type_trans(struct sk_buff *skb, struct net_device *dev);
@@ -3815,6 +3815,7 @@ static int __arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *rw_im
 {
 	int i, ret, nr_regs = m->nr_args, stack_size = 0;
 	int regs_off, func_meta_off, ip_off, run_ctx_off, arg_stack_off, rbx_off;
+	int tcc_off = 0;
 	struct bpf_tramp_nodes *fentry = &tnodes[BPF_TRAMP_FENTRY];
 	struct bpf_tramp_nodes *fexit = &tnodes[BPF_TRAMP_FEXIT];
 	struct bpf_tramp_nodes *fmod_ret = &tnodes[BPF_TRAMP_MODIFY_RETURN];
@@ -3865,11 +3866,12 @@ static int __arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *rw_im
 	 *
 	 * RBP - run_ctx_off [ bpf_tramp_run_ctx ]
 	 *
+	 * RBP - tcc_off   [ tail_call_cnt_ptr ]  BPF_TRAMP_F_TAIL_CALL_CTX
+	 *
 	 *                     [ stack_argN ]  BPF_TRAMP_F_CALL_ORIG
 	 *                     [ ...        ]
 	 *                     [ stack_arg2 ]
 	 * RBP - arg_stack_off [ stack_arg1 ]
-	 * RSP                 [ tail_call_cnt_ptr ] BPF_TRAMP_F_TAIL_CALL_CTX
 	 */
 
 	/* room for return value of orig_call or fentry prog */
@@ -3900,6 +3902,11 @@ static int __arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *rw_im
 	stack_size += (sizeof(struct bpf_tramp_run_ctx) + 7) & ~0x7;
 	run_ctx_off = stack_size;
 
+	if (flags & BPF_TRAMP_F_TAIL_CALL_CTX) {
+		stack_size += 8;
+		tcc_off = stack_size;
+	}
+
 	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;
@@ -3957,7 +3964,8 @@ static int __arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *rw_im
 		EMIT4(0x48, 0x83, 0xEC, stack_size);
 	}
 	if (flags & BPF_TRAMP_F_TAIL_CALL_CTX)
-		EMIT1(0x50);		/* push rax */
+		/* mov QWORD PTR [rbp - tcc_off], rax */
+		emit_stx(&prog, BPF_DW, BPF_REG_FP, BPF_REG_0, -tcc_off);
 	/* mov QWORD PTR [rbp - rbx_off], rbx */
 	emit_stx(&prog, BPF_DW, BPF_REG_FP, BPF_REG_6, -rbx_off);
 
@@ -4018,7 +4026,7 @@ static int __arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *rw_im
 			/* Before calling the original function, load the
 			 * tail_call_cnt_ptr from stack to rax.
 			 */
-			LOAD_TRAMP_TAIL_CALL_CNT_PTR(stack_size);
+			LOAD_TRAMP_TAIL_CALL_CNT_PTR(tcc_off);
 		}
 
 		if (flags & BPF_TRAMP_F_ORIG_STACK) {
@@ -4082,7 +4090,7 @@ static int __arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *rw_im
 		/* Before running the original function, load the
 		 * tail_call_cnt_ptr from stack to rax.
 		 */
-		LOAD_TRAMP_TAIL_CALL_CNT_PTR(stack_size);
+		LOAD_TRAMP_TAIL_CALL_CNT_PTR(tcc_off);
 	}
 
 	/* restore return value of orig_call or fentry prog back into RAX */
-- 
2.53.0-Meta

[PATCH bpf-next v2 02/11] selftests/bpf: Test trampoline stack args in a program that can tail call

[Yonghong Song]

Test the previous patch with a target program made of two call chains:

	run() -> call_stack() -> callee_stack(a, b, c, d, e, f, g, h)
	run() -> subprog_tail() -> bpf_tail_call()

callee_stack() takes eight u64 arguments, so g and h are passed on the
stack. Because run() can reach a tail call, a trampoline on any function
of the program keeps the tail call counter pointer. callee_stack()
itself does not lead to the tail call, which is why the verifier still
lets it take stack arguments.

Attach fentry and fexit programs to callee_stack(). The fentry program
checks that it sees g and h, and the test checks the value that
callee_stack() returns when the fexit trampoline calls it.

Without the previous patch, the function takes the counter pointer for
its first stack argument:

  test_stack_arg_tailcall:FAIL:stack ret unexpected stack ret: actual -359789677662885869 != expected 204
  test_stack_arg_tailcall:FAIL:stack fexit ret unexpected stack fexit ret: actual -359789677662885869 != expected 204

Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
---
 .../selftests/bpf/prog_tests/tracing_struct.c | 59 +++++++++++++++++++
 .../progs/tracing_stack_arg_tailcall_tgt.c    | 59 +++++++++++++++++++
 .../progs/tracing_stack_arg_tailcall_tracer.c | 23 ++++++++
 3 files changed, 141 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/progs/tracing_stack_arg_tailcall_tgt.c
 create mode 100644 tools/testing/selftests/bpf/progs/tracing_stack_arg_tailcall_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 15b95d0235b5..8086586d3e19 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_stack_arg_tailcall_tgt.skel.h"
+#include "tracing_stack_arg_tailcall_tracer.skel.h"
 
 static void test_struct_args(void)
 {
@@ -146,6 +148,61 @@ static void test_int128_args(void)
 #endif
 }
 
+/*
+ * A trampoline on a function of a tail_call_reachable program keeps the tail
+ * call counter pointer on its stack, and has to hand the function its stack
+ * arguments past it.
+ */
+static void test_stack_arg_tailcall(void)
+{
+#if defined(__x86_64__) || defined(__aarch64__)
+	struct tracing_stack_arg_tailcall_tracer *tracer = NULL;
+	struct tracing_stack_arg_tailcall_tgt *tgt;
+	LIBBPF_OPTS(bpf_test_run_opts, topts);
+	int err, fd;
+
+	tgt = tracing_stack_arg_tailcall_tgt__open_and_load();
+	if (!ASSERT_OK_PTR(tgt, "tgt__open_and_load"))
+		return;
+	if (!tgt->rodata->has_stack_arg) {
+		test__skip();
+		goto out;
+	}
+	fd = bpf_program__fd(tgt->progs.run);
+
+	tracer = tracing_stack_arg_tailcall_tracer__open();
+	if (!ASSERT_OK_PTR(tracer, "tracer__open"))
+		goto out;
+
+	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");
+	if (!ASSERT_OK(err, "set_attach_target"))
+		goto out;
+
+	err = tracing_stack_arg_tailcall_tracer__load(tracer);
+	if (!ASSERT_OK(err, "tracer__load"))
+		goto out;
+	err = tracing_stack_arg_tailcall_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->stack_g, 7, "stack:g");
+	ASSERT_EQ(tracer->bss->stack_h, 8, "stack:h");
+	ASSERT_EQ(tgt->bss->ret_stack, 204, "stack ret");
+	ASSERT_EQ(tracer->bss->stack_ret, 204, "stack fexit ret");
+
+out:
+	tracing_stack_arg_tailcall_tracer__destroy(tracer);
+	tracing_stack_arg_tailcall_tgt__destroy(tgt);
+#else
+	test__skip();
+#endif
+}
+
 static void test_union_args(void)
 {
 	struct tracing_struct *skel;
@@ -181,6 +238,8 @@ void test_tracing_struct(void)
 		test_struct_many_args();
 	if (test__start_subtest("int128_args"))
 		test_int128_args();
+	if (test__start_subtest("stack_arg_tailcall"))
+		test_stack_arg_tailcall();
 	if (test__start_subtest("union_args"))
 		test_union_args();
 }
diff --git a/tools/testing/selftests/bpf/progs/tracing_stack_arg_tailcall_tgt.c b/tools/testing/selftests/bpf/progs/tracing_stack_arg_tailcall_tgt.c
new file mode 100644
index 000000000000..5fde710c4e37
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/tracing_stack_arg_tailcall_tgt.c
@@ -0,0 +1,59 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2026 Meta Platforms, Inc. and affiliates. */
+#include <vmlinux.h>
+#include <bpf/bpf_helpers.h>
+
+struct {
+	__uint(type, BPF_MAP_TYPE_PROG_ARRAY);
+	__uint(max_entries, 1);
+	__uint(key_size, sizeof(__u32));
+	__uint(value_size, sizeof(__u32));
+} jmp_table SEC(".maps");
+
+__u64 base;
+__u64 ret_stack;
+
+#if defined(__clang__) && defined(__BPF_FEATURE_STACK_ARGUMENT)
+
+const volatile bool has_stack_arg = true;
+
+/* g and h are on the stack, for the x86-64 kernel convention too. */
+static __noinline __u64 callee_stack(__u64 a, __u64 b, __u64 c, __u64 d,
+				     __u64 e, __u64 f, __u64 g, __u64 h)
+{
+	return a + b * 2 + c * 3 + d * 4 + e * 5 + f * 6 + g * 7 + h * 8;
+}
+
+/* No stack args of its own, so it can share a program with a tail call. */
+static __noinline __u64 call_stack(__u64 x)
+{
+	return callee_stack(x + 1, x + 2, x + 3, x + 4, x + 5, x + 6, x + 7,
+			    x + 8);
+}
+
+#else
+
+const volatile bool has_stack_arg = false;
+
+#endif
+
+/* Makes the program tail_call_reachable, though callee_stack() is not. */
+static __noinline int subprog_tail(void *ctx)
+{
+	int ret = 0;
+
+	bpf_tail_call_static(ctx, &jmp_table, 0);
+	barrier_var(ret);
+	return ret;
+}
+
+SEC("syscall")
+int run(void *ctx)
+{
+#if defined(__clang__) && defined(__BPF_FEATURE_STACK_ARGUMENT)
+	ret_stack = call_stack(base);
+#endif
+	return subprog_tail(ctx);
+}
+
+char _license[] SEC("license") = "GPL";
diff --git a/tools/testing/selftests/bpf/progs/tracing_stack_arg_tailcall_tracer.c b/tools/testing/selftests/bpf/progs/tracing_stack_arg_tailcall_tracer.c
new file mode 100644
index 000000000000..1800e7b7afe2
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/tracing_stack_arg_tailcall_tracer.c
@@ -0,0 +1,23 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2026 Meta Platforms, Inc. and affiliates. */
+#include <vmlinux.h>
+#include <bpf/bpf_helpers.h>
+
+long stack_g, stack_h, stack_ret;
+
+SEC("fentry/callee_stack")
+int stack_entry(unsigned long long *ctx)
+{
+	stack_g = ctx[6];
+	stack_h = ctx[7];
+	return 0;
+}
+
+SEC("fexit/callee_stack")
+int stack_exit(unsigned long long *ctx)
+{
+	stack_ret = ctx[8];
+	return 0;
+}
+
+char _license[] SEC("license") = "GPL";
-- 
2.53.0-Meta

[PATCH bpf-next v2 03/11] 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 b4f0af2aeca4..00641d597f95 100644
--- a/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c
+++ b/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c
@@ -1047,6 +1047,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)
 {
@@ -1770,6 +1793,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 v2 04/11] bpf: Describe a BPF function to its trampoline as one u64 per slot

[Yonghong Song]

A trampoline on a BPF function, fentry, fexit or fsession 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 slot N
where the kernel convention passes its Nth u64 argument, in a register
or a stack slot, whatever the types of the arguments: the x86-64, arm64
and riscv JITs all place a stack argument that way. The model of the
function, distilled from its BTF, still carries those types, so each
arch places the arguments by its own rules for them. For a 16-byte
argument at an odd slot, such as s in

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

the arm64 trampoline reads s from x2 and x3, where the caller left it in
x1 and x2, and b from x4. And an x86-64 trampoline puts a 16-byte
argument at slot 6 wholly on the stack, while BPF code splits it between
r9 and the stack.

Rewrite the model of a target program's function in
bpf_check_attach_target() as one u64 argument per argument slot.
Every convention places N u64 arguments in slot order, so all of the
above take them from where BPF code put them, and no arch has to know
that the function is a BPF one.

The model no longer gives the size and flags of each argument, so an
arch that sign-extends a narrow signed argument, or clears the unused
bytes of a narrow stack argument, for the program does not do that for a
BPF function any more: the program gets the eightbyte BPF code passed,
as it does on x86-64, and fexit hands it back unchanged.

Fixes: afdbb4cdd3d2 ("bpf, arm64: Place trampoline arguments by the arm64 calling convention")
Fixes: 4dd676ca5b7a ("bpf: Support by-value struct arguments up to 16 bytes")
Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
---
 kernel/bpf/verifier.c | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 2f190abe3448..c7af14a3f0e8 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -22002,6 +22002,32 @@ static bool attach_uses_trampoline_retval(enum bpf_attach_type type)
 	}
 }
 
+/*
+ * BPF code passes argument slot N where the kernel convention passes its Nth
+ * u64 argument, so describe a BPF function to its trampoline as one u64 per
+ * slot, and every arch places its arguments right.
+ */
+static int flatten_bpf_func_model(struct bpf_verifier_log *log, const char *tname,
+				  struct btf_func_model *m)
+{
+	u32 i, nslots = 0;
+
+	for (i = 0; i < m->nr_args; i++)
+		nslots += btf_func_model_arg_slots(m, i);
+	if (nslots > MAX_BPF_FUNC_ARGS) {
+		bpf_log(log, "The function %s has %u argument slots. Too many.\n",
+			tname, nslots);
+		return -EINVAL;
+	}
+
+	for (i = 0; i < nslots; i++) {
+		m->arg_size[i] = sizeof(u64);
+		m->arg_flags[i] = 0;
+	}
+	m->nr_args = nslots;
+	return 0;
+}
+
 int bpf_check_attach_target(struct bpf_verifier_log *log,
 			    const struct bpf_prog *prog,
 			    const struct bpf_prog *tgt_prog,
@@ -22275,6 +22301,11 @@ 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;
+		if (tgt_prog) {
+			ret = flatten_bpf_func_model(log, tname, &tgt_info->fmodel);
+			if (ret < 0)
+				return ret;
+		}
 
 		if (tgt_info->fmodel.ret_size > 8 &&
 		    attach_uses_trampoline_retval(prog->expected_attach_type)) {
-- 
2.53.0-Meta

[PATCH bpf-next v2 05/11] 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.

A trampoline on a BPF function is not changed by this: since the
previous patch, the function's model has one u64 per argument slot,
which bpf_jit_place_args() places in slot order, so the trampoline still
takes the arguments where BPF code put them.

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

diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
index 3982507d54ca..9bf29dc39498 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;
 	int tcc_off = 0;
 	struct bpf_tramp_nodes *fentry = &tnodes[BPF_TRAMP_FENTRY];
@@ -3837,12 +3839,11 @@ 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, six in
+	 * registers and the rest on the stack, where bpf_jit_place_args()
+	 * puts them.
 	 */
+	nr_regs = bpf_jit_place_args(&x86_arg_abi, m, pos_of_slot);
 	if (nr_regs > MAX_BPF_FUNC_ARGS)
 		return -ENOTSUPP;
 
@@ -3854,11 +3855,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
 	 *
@@ -3868,10 +3869,10 @@ static int __arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *rw_im
 	 *
 	 * RBP - tcc_off   [ tail_call_cnt_ptr ]  BPF_TRAMP_F_TAIL_CALL_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
 	 */
 
 	/* room for return value of orig_call or fentry prog */
@@ -3882,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;
 
@@ -3909,7 +3910,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)]
@@ -3970,7 +3971,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) {
@@ -3978,7 +3979,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 */
@@ -4019,8 +4020,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
@@ -4073,7 +4074,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 v2 06/11] 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 8086586d3e19..a2da219e1476 100644
--- a/tools/testing/selftests/bpf/prog_tests/tracing_struct.c
+++ b/tools/testing/selftests/bpf/prog_tests/tracing_struct.c
@@ -119,9 +119,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;
@@ -141,6 +143,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 00641d597f95..38edb4addc5b 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 v2 07/11] 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.

Describing the functions to the trampoline by the types of their
arguments, rather than as one u64 per slot, 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 a2da219e1476..ad46eabe784f 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"
 #include "tracing_stack_arg_tailcall_tgt.skel.h"
 #include "tracing_stack_arg_tailcall_tracer.skel.h"
 
@@ -164,6 +166,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
+}
+
 /*
  * A trampoline on a function of a tail_call_reachable program keeps the tail
  * call counter pointer on its stack, and has to hand the function its stack
@@ -254,6 +326,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("stack_arg_tailcall"))
 		test_stack_arg_tailcall();
 	if (test__start_subtest("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

[PATCH bpf-next v2 08/11] bpf: Refuse a trampoline on an unreliable static function with stack args

[Yonghong Song]

When the BTF of a function in the target program is unreliable, the
model of the function given to its trampoline is MAX_BPF_FUNC_REG_ARGS
u64 arguments. The verifier takes a static function's stack arguments
from its BTF before it finds the BTF unreliable, though, so such a
function can still take more: one with six or more arguments, one of
them a plain pointer, for example. The trampoline saves and restores
five registers then. After an fentry program, the rest of the register
arguments, r9 on x86-64 and x5 to x7 on arm64, hold whatever
__bpf_prog_exit() left there, and for fexit the stack arguments are not
copied either, so the function goes on with garbage in arguments the
verifier checked, pointers included.

Refuse the attach when such a function has more argument slots in its
BTF than fit in registers. The main program is left alone: it takes only
its context, and the verifier gives it no stack arguments when its BTF
is unreliable.

Fixes: 0f6bd5e7a804 ("bpf: Support stack arguments for bpf functions")
Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
---
 kernel/bpf/verifier.c | 35 ++++++++++++++++++++++++++++++++++-
 1 file changed, 34 insertions(+), 1 deletion(-)

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index c7af14a3f0e8..1f24806aeba3 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -22028,6 +22028,33 @@ static int flatten_bpf_func_model(struct bpf_verifier_log *log, const char *tnam
 	return 0;
 }
 
+/*
+ * The verifier lets a static function use stack arguments counted from its
+ * BTF, even if that BTF turns out unreliable. A trampoline on the function is
+ * then given only MAX_BPF_FUNC_REG_ARGS u64 arguments and would lose the
+ * stack arguments, so such a function cannot be attached to.
+ */
+static int check_unreliable_func_args(struct bpf_verifier_log *log, struct btf *btf,
+				      const struct btf_type *func_proto, const char *tname)
+{
+	struct btf_func_model m;
+	u32 i, nslots = 0;
+
+	if (btf_distill_func_proto(NULL, btf, func_proto, tname, &m)) {
+		/* at least a slot for each argument */
+		nslots = btf_type_vlen(func_proto);
+	} else {
+		for (i = 0; i < m.nr_args; i++)
+			nslots += btf_func_model_arg_slots(&m, i);
+	}
+	if (nslots <= MAX_BPF_FUNC_REG_ARGS)
+		return 0;
+
+	bpf_log(log, "Cannot attach to static function %s: unreliable BTF and stack arguments\n",
+		tname);
+	return -EINVAL;
+}
+
 int bpf_check_attach_target(struct bpf_verifier_log *log,
 			    const struct bpf_prog *prog,
 			    const struct bpf_prog *tgt_prog,
@@ -22295,8 +22322,14 @@ int bpf_check_attach_target(struct bpf_verifier_log *log,
 		     prog->aux->saved_dst_attach_type != tgt_prog->expected_attach_type))
 			return -EINVAL;
 
-		if (tgt_prog && conservative)
+		if (tgt_prog && conservative) {
+			if (subprog > 0) {
+				ret = check_unreliable_func_args(log, btf, t, tname);
+				if (ret)
+					return ret;
+			}
 			t = NULL;
+		}
 
 		ret = btf_distill_func_proto(log, btf, t, tname, &tgt_info->fmodel);
 		if (ret < 0)
-- 
2.53.0-Meta

[PATCH bpf-next v2 09/11] selftests/bpf: Test trampolines on more BPF functions with stack args

[Yonghong Song]

Add two functions to the BPF target program of the int128_tgt_prog test.

callee_split() takes a struct holding an __int128 after five u64s, so
on x86-64 it is passed split between r9 and the stack. fentry and fexit
programs check that the trampoline takes it from there and hands it back
there.

callee_unreliable() takes six u64-sized arguments, the first a plain
pointer, which makes its BTF unreliable while the sixth is still passed
on the stack. The new tgt_prog_unreliable subtest checks that an fentry
program cannot be attached to it.

Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
---
 .../selftests/bpf/prog_tests/tracing_struct.c | 62 +++++++++++++++++++
 .../bpf/progs/tracing_struct_int128_tgt.c     | 29 ++++++++-
 .../bpf/progs/tracing_struct_int128_tracer.c  | 26 ++++++++
 3 files changed, 116 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/tracing_struct.c b/tools/testing/selftests/bpf/prog_tests/tracing_struct.c
index ad46eabe784f..e68fd5627ec1 100644
--- a/tools/testing/selftests/bpf/prog_tests/tracing_struct.c
+++ b/tools/testing/selftests/bpf/prog_tests/tracing_struct.c
@@ -194,9 +194,15 @@ static void test_int128_tgt_prog(void)
 							     "callee_stack");
 		err = err ?: bpf_program__set_attach_target(tracer->progs.stack_exit, fd,
 							     "callee_stack");
+		err = err ?: bpf_program__set_attach_target(tracer->progs.split_entry, fd,
+							     "callee_split");
+		err = err ?: bpf_program__set_attach_target(tracer->progs.split_exit, fd,
+							     "callee_split");
 	} else {
 		bpf_program__set_autoload(tracer->progs.stack_entry, false);
 		bpf_program__set_autoload(tracer->progs.stack_exit, false);
+		bpf_program__set_autoload(tracer->progs.split_entry, false);
+		bpf_program__set_autoload(tracer->progs.split_exit, false);
 	}
 	if (!ASSERT_OK(err, "set_attach_target"))
 		goto out;
@@ -226,6 +232,13 @@ static void test_int128_tgt_prog(void)
 		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");
+
+		ASSERT_EQ(tracer->bss->split_e, 5, "split:e");
+		ASSERT_EQ(tracer->bss->split_lo, 8, "split:h.lo");
+		ASSERT_EQ(tracer->bss->split_hi, 9, "split:h.hi");
+		ASSERT_EQ(tracer->bss->split_i, 10, "split:i");
+		ASSERT_EQ(tgt->bss->ret_split, 246, "split ret");
+		ASSERT_EQ(tracer->bss->split_ret, 246, "split fexit ret");
 	}
 
 out:
@@ -241,6 +254,53 @@ static void test_int128_tgt_prog(void)
  * call counter pointer on its stack, and has to hand the function its stack
  * arguments past it.
  */
+/*
+ * A static function whose BTF is unreliable gets a trampoline that only knows
+ * of the register arguments, so one taking stack arguments must be refused.
+ */
+static void test_tgt_prog_unreliable(void)
+{
+#if defined(__x86_64__) || defined(__aarch64__)
+	struct tracing_struct_int128_tracer *tracer = NULL;
+	struct tracing_struct_int128_tgt *tgt;
+	char log_buf[4096] = {};
+	struct bpf_program *prog;
+	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_stack_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;
+
+	bpf_object__for_each_program(prog, tracer->obj)
+		bpf_program__set_autoload(prog, false);
+	bpf_program__set_autoload(tracer->progs.unreliable_entry, true);
+	bpf_program__set_log_buf(tracer->progs.unreliable_entry, log_buf, sizeof(log_buf));
+	err = bpf_program__set_attach_target(tracer->progs.unreliable_entry, fd,
+					     "callee_unreliable");
+	if (!ASSERT_OK(err, "set_attach_target"))
+		goto out;
+
+	err = tracing_struct_int128_tracer__load(tracer);
+	ASSERT_ERR(err, "tracer__load");
+	ASSERT_HAS_SUBSTR(log_buf, "unreliable BTF and stack arguments", "log");
+
+out:
+	tracing_struct_int128_tracer__destroy(tracer);
+	tracing_struct_int128_tgt__destroy(tgt);
+#else
+	test__skip();
+#endif
+}
+
 static void test_stack_arg_tailcall(void)
 {
 #if defined(__x86_64__) || defined(__aarch64__)
@@ -328,6 +388,8 @@ void test_tracing_struct(void)
 		test_int128_args();
 	if (test__start_subtest("int128_tgt_prog"))
 		test_int128_tgt_prog();
+	if (test__start_subtest("tgt_prog_unreliable"))
+		test_tgt_prog_unreliable();
 	if (test__start_subtest("stack_arg_tailcall"))
 		test_stack_arg_tailcall();
 	if (test__start_subtest("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
index 2ec8f741d48b..d1f9c3ccc99c 100644
--- a/tools/testing/selftests/bpf/progs/tracing_struct_int128_tgt.c
+++ b/tools/testing/selftests/bpf/progs/tracing_struct_int128_tgt.c
@@ -8,7 +8,7 @@ struct int128_arg {
 };
 
 __u64 base;
-__u64 ret_odd, ret_stack;
+__u64 ret_odd, ret_stack, ret_split, ret_unreliable;
 
 #if defined(__clang__)
 
@@ -44,6 +44,25 @@ static __noinline __u64 callee_stack(__u64 a, __u64 b, __u64 c, __u64 d,
 	       i * 10;
 }
 
+/* BPF code splits h between the last argument register and the stack. */
+static __noinline __u64 callee_split(__u64 a, __u64 b, __u64 c, __u64 d,
+				     __u64 e, struct int128_arg h, __u64 i)
+{
+	return a + b * 2 + c * 3 + d * 4 + e * 5 + (__u64)h.v * 6 +
+	       (__u64)((unsigned __int128)h.v >> 64) * 7 + i * 8;
+}
+
+/*
+ * A plain pointer makes the BTF of a static function unreliable, but f is
+ * still passed on the stack. The store keeps p a pointer.
+ */
+static __noinline __u64 callee_unreliable(__u64 *p, __u64 b, __u64 c, __u64 d,
+					  __u64 e, __u64 f)
+{
+	*p += 1;
+	return *p + b * 2 + c * 3 + d * 4 + e * 5 + f * 6;
+}
+
 #else
 
 const volatile bool has_stack_arg = false;
@@ -60,6 +79,14 @@ int run(void *ctx)
 #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);
+	ret_split = callee_split(base + 1, base + 2, base + 3, base + 4,
+				 base + 5, v, base + 10);
+	{
+		__u64 x = base;
+
+		ret_unreliable = callee_unreliable(&x, base + 2, base + 3,
+						   base + 4, base + 5, base + 6);
+	}
 #endif
 #endif
 	return 0;
diff --git a/tools/testing/selftests/bpf/progs/tracing_struct_int128_tracer.c b/tools/testing/selftests/bpf/progs/tracing_struct_int128_tracer.c
index 133f24fc9158..612d6f578ecb 100644
--- a/tools/testing/selftests/bpf/progs/tracing_struct_int128_tracer.c
+++ b/tools/testing/selftests/bpf/progs/tracing_struct_int128_tracer.c
@@ -41,4 +41,30 @@ int stack_exit(unsigned long long *ctx)
 	return 0;
 }
 
+long split_e, split_lo, split_hi, split_i, split_ret;
+
+SEC("fentry/callee_split")
+int split_entry(unsigned long long *ctx)
+{
+	split_e = ctx[4];
+	split_lo = ctx[5];
+	split_hi = ctx[6];
+	split_i = ctx[7];
+	return 0;
+}
+
+SEC("fexit/callee_split")
+int split_exit(unsigned long long *ctx)
+{
+	split_ret = ctx[8];
+	return 0;
+}
+
+/* Refused: the trampoline would lose the stack argument. */
+SEC("?fentry/callee_unreliable")
+int unreliable_entry(unsigned long long *ctx)
+{
+	return 0;
+}
+
 char _license[] SEC("license") = "GPL";
-- 
2.53.0-Meta

[PATCH bpf-next v2 10/11] bpf, x86: Pass a kfunc's 16-byte aligned stack argument 16-byte aligned

[Yonghong Song]

An argument aligned to 16 bytes, an __int128 or a struct holding one,
goes in a 16-byte aligned stack slot in the x86-64 calling convention.
The kernel keeps its stack only 8-byte aligned, being built with
-mpreferred-stack-boundary=3, yet a caller still realigns RSP before it
passes such an argument on the stack, and the callee can take the
alignment for granted. For

	u64 f(u64 a, u64 b, u64 c, u64 d, u64 e, u64 f, __int128 g)

both GCC and clang fold ((unsigned long)&g & 15) == 0 to true, and GCC
passes &g on as an aligned __int128 pointer without copying g.

The JIT places a kfunc's stack arguments at the offsets from RSP that
the convention gives them, holes included, but RSP itself is only 8-byte
aligned in a BPF program, so a kfunc can get such an argument at an
address that is not 16-byte aligned.

When a kfunc call passes an argument aligned to 16 bytes on the stack,
test RSP before the call, and if it is off by 8, copy the stack argument
slots to the first 16-byte aligned place below them. A push of each slot
does that in four bytes: it takes the address of its operand before it
moves RSP, so one push instruction, repeated, copies the slots from the
top down. After the call, put RSP back from RBP. The arm64 JIT keeps SP
16-byte aligned, so only x86-64 needs this.

bpf_jit_stack_slots_to_align(), next to bpf_jit_place_args(), returns
how many stack slots a call has to realign, from where its arguments
are placed. The x86-64 JIT now places a kfunc's arguments once, for it
and for bpf_jit_plan_slot_moves(), the half of bpf_jit_plan_arg_moves()
that plans the moves from a given placement. bpf_jit_plan_arg_moves()
keeps placing the arguments itself, for arm64.

Fixes: a3195057406f ("bpf, x86: Move kfunc arguments into the x86-64 calling convention")
Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
---
 arch/x86/net/bpf_jit_comp.c | 65 ++++++++++++++++++++++++++++++++++---
 include/linux/filter.h      |  5 +++
 kernel/bpf/core.c           | 50 +++++++++++++++++++++++-----
 3 files changed, 107 insertions(+), 13 deletions(-)

diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
index 9bf29dc39498..ad1e1187154f 100644
--- a/arch/x86/net/bpf_jit_comp.c
+++ b/arch/x86/net/bpf_jit_comp.c
@@ -1918,17 +1918,19 @@ static s32 x86_arg_stack_off(u8 pos)
 
 /*
  * Move the arguments the x86-64 ABI places somewhere other than the argument
- * slot the BPF calling convention gave them. @stack_base addresses the
+ * slot the BPF calling convention gave them, @pos_of_slot being where
+ * bpf_jit_place_args() put each of the @nslots slots. @stack_base addresses the
  * outgoing stack argument area from RBP. Return the number of emitted bytes.
  */
-static int emit_kfunc_arg_moves(const struct btf_func_model *fm, s32 stack_base, u8 **pprog)
+static int emit_kfunc_arg_moves(const u8 *pos_of_slot, u32 nslots, s32 stack_base,
+				u8 **pprog)
 {
 	struct bpf_jit_arg_move moves[BPF_JIT_MAX_ARG_MOVES];
 	const u8 nreg = x86_arg_abi.nr_arg_regs;
 	u8 *prog = *pprog, *start = prog;
 	u32 i, n;
 
-	n = bpf_jit_plan_arg_moves(&x86_arg_abi, fm, moves);
+	n = bpf_jit_plan_slot_moves(pos_of_slot, nslots, moves);
 
 	for (i = 0; i < n; i++) {
 		u8 dst = moves[i].dst, src = moves[i].src, reg;
@@ -1965,6 +1967,36 @@ static int emit_kfunc_arg_moves(const struct btf_func_model *fm, s32 stack_base,
 	return prog - start;
 }
 
+/*
+ * When RSP is not 16-byte aligned, copy the @n on-stack argument slots at RSP
+ * to the first 16-byte aligned place below them. A push takes the address of
+ * its memory operand before it moves RSP, so the same push copies each slot in
+ * turn, from the top down. The caller puts RSP back after the call. Return the
+ * number of emitted bytes.
+ */
+static int emit_kfunc_align_stack_args(u32 n, u8 **pprog)
+{
+	u8 *prog = *pprog, *start = prog, *skip;
+	/* a kfunc takes MAX_BPF_FUNC_ARGS slots at most: disp fits in a disp8 */
+	u8 disp = (n - 1) * 8;
+	u32 i;
+
+	EMIT4(0x40, 0xF6, 0xC4, 0x08);		/* test spl, 8 */
+	EMIT2(X86_JE, 0);			/* je over the copy */
+	skip = prog;
+	if (!(n & 1)) {
+		/* an even number of slots keeps RSP's alignment */
+		EMIT4(0x48, 0x83, 0xEC, 8);	/* sub rsp, 8 */
+		disp += 8;
+	}
+	for (i = 0; i < n; i++)
+		EMIT4(0xFF, 0x74, 0x24, disp);	/* push qword ptr [rsp + disp] */
+	skip[-1] = prog - skip;
+
+	*pprog = prog;
+	return prog - start;
+}
+
 /*
  * Rebase the __arena args of a kfunc call to arena kernel addresses,
  * rN = kern_vm_start + (u32)rN, with R12 holding kern_vm_start. A nullable
@@ -2953,6 +2985,8 @@ static int do_jit(struct bpf_verifier_env *env, struct bpf_prog *bpf_prog, int *
 			/* call */
 		case BPF_JMP | BPF_CALL: {
 			const struct btf_func_model *fm = NULL;
+			u8 pos_of_slot[MAX_BPF_FUNC_ARG_SLOTS];
+			u32 nslots, align_slots = 0;
 
 			func = (u8 *) __bpf_call_base + imm32;
 			if (src_reg == BPF_PSEUDO_CALL && tail_call_reachable) {
@@ -2969,8 +3003,20 @@ static int do_jit(struct bpf_verifier_env *env, struct bpf_prog *bpf_prog, int *
 				if (err < 0)
 					return err;
 				ip += err;
-				ip += emit_kfunc_arg_moves(fm, outgoing_arg_base -
-							   outgoing_rsp, &prog);
+				nslots = bpf_jit_place_args(&x86_arg_abi, fm, pos_of_slot);
+				ip += emit_kfunc_arg_moves(pos_of_slot, nslots,
+							   outgoing_arg_base - outgoing_rsp,
+							   &prog);
+				/*
+				 * The kernel keeps the stack only 8-byte aligned,
+				 * but a caller realigns RSP to pass an argument
+				 * aligned to 16 bytes on the stack, and the callee
+				 * may rely on its slot's alignment.
+				 */
+				align_slots = bpf_jit_stack_slots_to_align(&x86_arg_abi, fm,
+									   pos_of_slot);
+				if (align_slots)
+					ip += emit_kfunc_align_stack_args(align_slots, &prog);
 			}
 			if (func == (u8 *)arch_bpf_timed_may_goto) {
 				u32 fp = priv_frame_ptr ? X86_REG_R9 : BPF_REG_FP;
@@ -2993,6 +3039,15 @@ static int do_jit(struct bpf_verifier_env *env, struct bpf_prog *bpf_prog, int *
 				return -EINVAL;
 			if (priv_frame_ptr)
 				pop_r9(&prog);
+			if (align_slots) {
+				s32 off = outgoing_arg_base - outgoing_rsp;
+
+				/* lea rsp, [rbp + off], wherever the copy left it */
+				if (is_imm8(off))
+					EMIT4(0x48, 0x8D, 0x65, (u8)off);
+				else
+					EMIT3_off32(0x48, 0x8D, 0xA5, off);
+			}
 			/*
 			 * A kfunc returning more than 8 bytes hands the second
 			 * half back in RDX (the native ABI's second return reg),
diff --git a/include/linux/filter.h b/include/linux/filter.h
index 9339c6131f8f..55875f3a1510 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h
@@ -1285,9 +1285,14 @@ struct bpf_jit_arg_move {
 	u8 src;
 };
 
+u32 bpf_jit_plan_slot_moves(const u8 *pos_of_slot, u32 nslots,
+			    struct bpf_jit_arg_move *moves);
 u32 bpf_jit_plan_arg_moves(const struct bpf_jit_arg_abi *abi,
 			   const struct btf_func_model *fm,
 			   struct bpf_jit_arg_move *moves);
+u32 bpf_jit_stack_slots_to_align(const struct bpf_jit_arg_abi *abi,
+				 const struct btf_func_model *fm,
+				 const u8 *pos_of_slot);
 u64 bpf_arch_uaddress_limit(void);
 void arch_bpf_stack_walk(bool (*consume_fn)(void *cookie, u64 ip, u64 sp, u64 bp), void *cookie);
 u64 arch_bpf_timed_may_goto(void);
diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
index 05c89396119a..3d6d504cdb5f 100644
--- a/kernel/bpf/core.c
+++ b/kernel/bpf/core.c
@@ -3379,15 +3379,10 @@ u32 bpf_jit_place_args(const struct bpf_jit_arg_abi *abi,
 	return slot;
 }
 
-u32 bpf_jit_plan_arg_moves(const struct bpf_jit_arg_abi *abi,
-			   const struct btf_func_model *fm,
-			   struct bpf_jit_arg_move *moves)
+u32 bpf_jit_plan_slot_moves(const u8 *pos_of_slot, u32 nslots,
+			    struct bpf_jit_arg_move *moves)
 {
-	u8 pos_of_slot[MAX_BPF_FUNC_ARG_SLOTS];
-	u32 nslots, n = 0, s, back;
-
-	nslots = bpf_jit_place_args(abi, fm, pos_of_slot);
-	back = nslots;
+	u32 n = 0, s, back = nslots;
 
 	/*
 	 * An argument is two eightbytes at most, so it frees one register at
@@ -3420,6 +3415,45 @@ u32 bpf_jit_plan_arg_moves(const struct bpf_jit_arg_abi *abi,
 	return n;
 }
 
+u32 bpf_jit_plan_arg_moves(const struct bpf_jit_arg_abi *abi,
+			   const struct btf_func_model *fm,
+			   struct bpf_jit_arg_move *moves)
+{
+	u8 pos_of_slot[MAX_BPF_FUNC_ARG_SLOTS];
+	u32 nslots;
+
+	nslots = bpf_jit_place_args(abi, fm, pos_of_slot);
+	return bpf_jit_plan_slot_moves(pos_of_slot, nslots, moves);
+}
+
+/*
+ * Return how many stack slots a call passes when one of its stack arguments is
+ * aligned to 16 bytes, or 0 otherwise. @pos_of_slot is where
+ * bpf_jit_place_args() put each argument slot. Such an argument goes in a
+ * 16-byte aligned stack slot, so a JIT whose stack is only 8-byte aligned has
+ * to realign that many slots before the call.
+ */
+u32 bpf_jit_stack_slots_to_align(const struct bpf_jit_arg_abi *abi,
+				 const struct btf_func_model *fm,
+				 const u8 *pos_of_slot)
+{
+	u32 i, k, slot = 0, top = 0;
+	bool align16 = false;
+
+	for (i = 0; i < fm->nr_args; i++) {
+		u32 nslots = btf_func_model_arg_slots(fm, i);
+
+		if ((fm->arg_flags[i] & BTF_FMODEL_ALIGN16_ARG) &&
+		    pos_of_slot[slot] >= abi->nr_arg_regs)
+			align16 = true;
+		for (k = 0; k < nslots; k++, slot++)
+			if (pos_of_slot[slot] >= abi->nr_arg_regs)
+				top = max_t(u32, top, pos_of_slot[slot] - abi->nr_arg_regs + 1);
+	}
+
+	return align16 ? top : 0;
+}
+
 bool __weak bpf_jit_supports_stack_args(void)
 {
 	return false;
-- 
2.53.0-Meta

[PATCH bpf-next v2 11/11] selftests/bpf: Test that a kfunc gets a 16-byte aligned __int128 aligned

[Yonghong Song]

Add bpf_kfunc_call_test_i128_arg_align(), which takes an __int128 after
seven u64s, in the stack slot after a hole, and returns the low four
bits of its address. OPTIMIZER_HIDE_VAR() keeps the compiler from taking
the alignment the ABI promises for granted and folding them to 0.

Call it from two subprograms whose frames are 8 bytes apart, so that one
of the calls starts with RSP off by 8 whatever alignment the program
was given, and check that both find the __int128 16-byte aligned.
Without the previous patch, the x86-64 JIT fails it with

	run_subtest:FAIL:1552 Unexpected retval: 2 != 0
	aggregate_arg/aggregate_arg_kfunc_int128_align:FAIL

Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
---
 .../selftests/bpf/progs/aggregate_arg_kfunc.c | 42 +++++++++++++++++++
 .../selftests/bpf/test_kmods/bpf_testmod.c    | 15 +++++++
 .../bpf/test_kmods/bpf_testmod_kfunc.h        |  2 +
 3 files changed, 59 insertions(+)

diff --git a/tools/testing/selftests/bpf/progs/aggregate_arg_kfunc.c b/tools/testing/selftests/bpf/progs/aggregate_arg_kfunc.c
index c87e886423f4..3c3bcb3a02d5 100644
--- a/tools/testing/selftests/bpf/progs/aggregate_arg_kfunc.c
+++ b/tools/testing/selftests/bpf/progs/aggregate_arg_kfunc.c
@@ -222,6 +222,48 @@ int aggregate_arg_kfunc_int128_pad(struct __sk_buff *skb)
 	return 0;
 }
 
+/*
+ * The kfunc returns the low four bits of the address of v, which both
+ * conventions pass in a 16-byte aligned stack slot. A program's frame is
+ * only 8-byte aligned on x86-64, so call it from two frames 8 bytes apart:
+ * one of them starts the call off by 8, whatever the program was given.
+ */
+static __noinline __u64 int128_align_frame1(__u64 a, __u64 b)
+{
+	volatile __u64 pad[1];
+	u128 v = ((u128)a << 64) | b;
+
+	pad[0] = a;
+	return bpf_kfunc_call_test_i128_arg_align(1, 2, 3, 4, 5, 6, pad[0], v);
+}
+
+static __noinline __u64 int128_align_frame2(__u64 a, __u64 b)
+{
+	volatile __u64 pad[2];
+	u128 v = ((u128)a << 64) | b;
+
+	pad[0] = a;
+	pad[1] = b;
+	return bpf_kfunc_call_test_i128_arg_align(1, 2, 3, 4, 5, pad[1], pad[0], v);
+}
+
+SEC("tc")
+__arch_x86_64 __arch_arm64
+__load_if_JITed()
+__success __retval(0)
+int aggregate_arg_kfunc_int128_align(struct __sk_buff *skb)
+{
+	__u64 a = skb->len ^ MIX_A;
+	__u64 b = skb->len ^ MIX_B;
+
+	if (int128_align_frame1(a, b))
+		return 1;
+	if (int128_align_frame2(a, b))
+		return 2;
+
+	return 0;
+}
+
 #endif /* __SIZEOF_INT128__ */
 
 #endif
diff --git a/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c b/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c
index 38edb4addc5b..8550897fdfa6 100644
--- a/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c
+++ b/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c
@@ -1120,6 +1120,20 @@ __bpf_kfunc u64 bpf_kfunc_call_test_i128_arg_pad(u64 a, u64 b, u64 c, u64 d, u64
 	       (u64)v * 8 + (u64)((unsigned __int128)v >> 64) * 9;
 }
 
+/*
+ * The low four bits of the address of v, which follows a stack slot hole: 0 if
+ * the caller passed it in a 16-byte aligned slot, as the x86-64 ABI asks.
+ */
+__bpf_kfunc u64 bpf_kfunc_call_test_i128_arg_align(u64 a, u64 b, u64 c, u64 d, u64 e,
+						   u64 f, u64 g, __int128 v)
+{
+	unsigned long addr = (unsigned long)&v;
+
+	/* or the compiler takes the alignment the ABI promises for granted */
+	OPTIMIZER_HIDE_VAR(addr);
+	return addr & 15;
+}
+
 __bpf_kfunc u64 bpf_kfunc_call_test_pair_arg_nofit(u64 a, u64 b, u64 c, u64 d,
 						   struct prog_test_pair_arg s)
 {
@@ -1840,6 +1854,7 @@ 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_i128_arg_align)
 BTF_ID_FLAGS(func, bpf_kfunc_call_test_pair_arg_nofit)
 BTF_ID_FLAGS(func, bpf_kfunc_call_test_pair_arg_tail)
 BTF_ID_FLAGS(func, bpf_kfunc_call_test_pair_arg_split8)
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 a6a93c901416..da0c9b0c38a4 100644
--- a/tools/testing/selftests/bpf/test_kmods/bpf_testmod_kfunc.h
+++ b/tools/testing/selftests/bpf/test_kmods/bpf_testmod_kfunc.h
@@ -249,6 +249,8 @@ __u64 bpf_kfunc_call_test_i128_arg_stack_hole(__u64 a, __u64 b, __u64 c, __u64 d
 					      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;
+__u64 bpf_kfunc_call_test_i128_arg_align(__u64 a, __u64 b, __u64 c, __u64 d, __u64 e,
+					 __u64 f, __u64 g, __int128 v) __ksym;
 #endif
 __u64 bpf_kfunc_call_test_pair_arg_nofit(__u64 a, __u64 b, __u64 c, __u64 d,
 					 struct prog_test_pair_arg s) __ksym;
-- 
2.53.0-Meta

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] bpf: endianness assumption causes selftest failure on aarch64 big-endian
--

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

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

This commit adds tests for bpf_testmod functions taking a 16-byte aligned
struct containing an __int128. It checks that the BPF trampoline packs
these arguments correctly according to the BPF calling convention.

> diff --git a/tools/testing/selftests/bpf/prog_tests/tracing_struct.c b/tools/testing/selftests/bpf/prog_tests/tracing_struct.c
> index 8086586d3e196..a2da219e14769 100644
> --- a/tools/testing/selftests/bpf/prog_tests/tracing_struct.c
> +++ b/tools/testing/selftests/bpf/prog_tests/tracing_struct.c
[ ... ]
> @@ -141,6 +143,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");

[Severity: Low]
Does this code assume a little-endian memory layout for the __int128
argument?

The test passes a 16-byte struct containing ((__int128)9 << 64) | 8.
On big-endian architectures like aarch64 (when configured with
CONFIG_CPU_BIG_ENDIAN), the most significant 64 bits (9) are placed at the
lowest address, and the least significant bits (8) at the higher address.
This hardcoded expectation for the little-endian layout appears to cause the
test_int128_args selftest to fail its assertions on big-endian systems.

> +	ASSERT_EQ(skel->bss->s_i, 10, "s:i");
> +	ASSERT_EQ(skel->bss->s_ret, 385, "s ret");
[ ... ]
> 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: Low]
Is this direct array indexing in test_int128_stack_fentry() subject to the
same endianness assumption?

When the BPF trampoline projects the struct onto the u64 ctx[] array on a
big-endian system, ctx[7] receives 9 and ctx[8] receives 8, rather than the
little-endian projection expected here.

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

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

Re: [PATCH bpf-next v2 09/11] selftests/bpf: Test trampolines on more BPF functions with stack args

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] bpf: check_unreliable_func_args incorrectly counts slots, allowing corrupting trampoline
--

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

selftests/bpf: Test trampolines on more BPF functions with stack args

Add two functions to the BPF target program of the int128_tgt_prog test.
callee_split() takes a struct holding an __int128 after five u64s, so
on x86-64 it is passed split between r9 and the stack.

> diff --git a/tools/testing/selftests/bpf/prog_tests/tracing_struct.c b/tools/testing/selftests/bpf/prog_tests/tracing_struct.c
> index ad46eabe784f8..e68fd5627ec1c 100644
> --- a/tools/testing/selftests/bpf/prog_tests/tracing_struct.c
> +++ b/tools/testing/selftests/bpf/prog_tests/tracing_struct.c
[ ... ]
> +/*
> + * A static function whose BTF is unreliable gets a trampoline that only knows
> + * of the register arguments, so one taking stack arguments must be refused.
> + */
> +static void test_tgt_prog_unreliable(void)
> +{
[ ... ]
> +	err = bpf_program__set_attach_target(tracer->progs.unreliable_entry, fd,
> +					     "callee_unreliable");
> +	if (!ASSERT_OK(err, "set_attach_target"))
> +		goto out;
> +
> +	err = tracing_struct_int128_tracer__load(tracer);
> +	ASSERT_ERR(err, "tracer__load");
> +	ASSERT_HAS_SUBSTR(log_buf, "unreliable BTF and stack arguments", "log");

[Severity: High]
While this test accurately checks callee_unreliable() which takes 6 scalar
arguments, is there a potential issue in the verifier's
check_unreliable_func_args() when dealing with struct arguments that occupy
multiple slots?

In kernel/bpf/verifier.c:check_unreliable_func_args(), if
btf_distill_func_proto() fails (e.g. because an argument is a struct larger
than 16 bytes), the fallback path counts the number of arguments instead
of 8-byte slots:

	if (btf_distill_func_proto(NULL, btf, func_proto, tname, &m)) {
		/* at least a slot for each argument */
		nslots = btf_type_vlen(func_proto);
	}

For a BPF subprogram taking a 32-byte struct by value and 4 other arguments
(5 arguments total), the actual slot count is 1 + 1 + 1 + 1 + 4 = 8 slots
(meaning 3 slots are on the stack). But the fallback calculates nslots = 5.

Since 5 <= MAX_BPF_FUNC_REG_ARGS (5), check_unreliable_func_args()
incorrectly returns 0, permitting attachment.

The generated trampoline will not save or restore the stack arguments.

Does this cause the target BPF subprogram to read uninitialized stack
memory from the trampoline frame as its arguments, leading to data corruption
and undefined behavior?

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

Re: [PATCH bpf-next v2 09/11] selftests/bpf: Test trampolines on more BPF functions with stack args

[bot+bpf-ci]

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

> selftests/bpf: Test trampolines on more BPF functions with stack args

TL;DR: Nit only: the callee_split() comment in the selftest says h is split
between register and stack, but that is x86-64 specific; on arm64 nothing is
split. Suggests saying so.

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

[ ... ]

> @@ -44,6 +44,25 @@ static __noinline __u64 callee_stack(__u64 a, __u64 b, __u64 c, __u64 d,
>  	       i * 10;
>  }
>
> +/* BPF code splits h between the last argument register and the stack. */
> +static __noinline __u64 callee_split(__u64 a, __u64 b, __u64 c, __u64 d,
> +				     __u64 e, struct int128_arg h, __u64 i)
> +{
> +	return a + b * 2 + c * 3 + d * 4 + e * 5 + (__u64)h.v * 6 +
> +	       (__u64)((unsigned __int128)h.v >> 64) * 7 + i * 8;
> +}

This isn't a bug, but is this comment accurate for every architecture this
test runs on?

test_int128_tgt_prog() is built for x86-64 and for arm64, and a..e already
occupy R1-R5 in BPF terms, so h and i are each stack arguments as far as
the BPF ISA is concerned.

The split between a register and the stack only comes from the x86-64 JIT,
which maps the first stack argument slot to r9:

arch/x86/net/bpf_jit_comp.c:x86_arg_reg[] = { BPF_REG_1, ..., BPF_REG_5,
X86_REG_R9 }

On arm64 the first three stack argument slots are mapped to x5-x7, so h
would land in x5/x6 and i in x7, with nothing split.

The commit message already says "on x86-64 it is passed split between r9
and the stack", and the comment on the neighbouring callee_stack() names
x86-64 and arm64 separately.

Could this comment say that the split is x86-64 specific, for example "x86-64
BPF code passes h in r9 and its first stack slot"?


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

Re: [PATCH bpf-next v2 05/11] bpf, x86: Place trampoline arguments by the x86-64 calling convention

[bot+bpf-ci]

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

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

TL;DR: Nits only: two comments in the x86 trampoline code (save_args() and
__arch_prepare_bpf_trampoline()) could put the opening /* on its own line.

> diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
> --- a/arch/x86/net/bpf_jit_comp.c
> +++ b/arch/x86/net/bpf_jit_comp.c

[ ... ]

> @@ -3472,8 +3479,35 @@ static void save_args(const struct btf_func_model *m, u8 **prog,

[ ... ]

> +		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.
> +				 */

This isn't a bug, but this comment is re-wrapped in the new per-slot loop in
save_args() with text on the same line as the opening /*.  Could it use the
form with the opening /* on its own line, as is preferred in BPF code?

	/*
	 * Only copy the arguments on-stack to ...
	 */

[ ... ]

> @@ -3837,12 +3839,11 @@ 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, six in
> +	 * registers and the rest on the stack, where bpf_jit_place_args()
> +	 * puts them.
>  	 */
> +	nr_regs = bpf_jit_place_args(&x86_arg_abi, m, pos_of_slot);
>  	if (nr_regs > MAX_BPF_FUNC_ARGS)
>  		return -ENOTSUPP;

This isn't a bug, but the same applies to this reworded comment in
__arch_prepare_bpf_trampoline().  Could it also be changed to the form with
the opening /* on its own line?


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

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

[Yonghong Song]

On 10/13/26 2:43 AM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Low] bpf: endianness assumption causes selftest failure on aarch64 big-endian
> --
>
> commit df1efd6b9c0e934bad453d07e6efe674bda8e1e6
> Author: Yonghong Song <yonghong.song@linux.dev>
>
> selftests/bpf: Test trampoline args around a 16-byte aligned stack arg
>
> This commit adds tests for bpf_testmod functions taking a 16-byte aligned
> struct containing an __int128. It checks that the BPF trampoline packs
> these arguments correctly according to the BPF calling convention.
>
>> diff --git a/tools/testing/selftests/bpf/prog_tests/tracing_struct.c b/tools/testing/selftests/bpf/prog_tests/tracing_struct.c
>> index 8086586d3e196..a2da219e14769 100644
>> --- a/tools/testing/selftests/bpf/prog_tests/tracing_struct.c
>> +++ b/tools/testing/selftests/bpf/prog_tests/tracing_struct.c
> [ ... ]
>> @@ -141,6 +143,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");
> [Severity: Low]
> Does this code assume a little-endian memory layout for the __int128
> argument?
>
> The test passes a 16-byte struct containing ((__int128)9 << 64) | 8.
> On big-endian architectures like aarch64 (when configured with
> CONFIG_CPU_BIG_ENDIAN), the most significant 64 bits (9) are placed at the
> lowest address, and the least significant bits (8) at the higher address.
> This hardcoded expectation for the little-endian layout appears to cause the
> test_int128_args selftest to fail its assertions on big-endian systems.

Ok, will fix due to possible arm64 big endian.

>
>> +	ASSERT_EQ(skel->bss->s_i, 10, "s:i");
>> +	ASSERT_EQ(skel->bss->s_ret, 385, "s ret");
> [ ... ]
>> 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: Low]
> Is this direct array indexing in test_int128_stack_fentry() subject to the
> same endianness assumption?
>
> When the BPF trampoline projects the struct onto the u64 ctx[] array on a
> big-endian system, ctx[7] receives 9 and ctx[8] receives 8, rather than the
> little-endian projection expected here.

Ok, will fix due to possible arm64 big endian.

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

Re: [PATCH bpf-next v2 09/11] selftests/bpf: Test trampolines on more BPF functions with stack args

[Yonghong Song]

On 10/13/26 2:51 AM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] bpf: check_unreliable_func_args incorrectly counts slots, allowing corrupting trampoline
> --
>
> commit c87f5bcaff5cc54c864fa434bbfefe8d6bff2bbc
> Author: Yonghong Song <yonghong.song@linux.dev>
>
> selftests/bpf: Test trampolines on more BPF functions with stack args
>
> Add two functions to the BPF target program of the int128_tgt_prog test.
> callee_split() takes a struct holding an __int128 after five u64s, so
> on x86-64 it is passed split between r9 and the stack.
>
>> diff --git a/tools/testing/selftests/bpf/prog_tests/tracing_struct.c b/tools/testing/selftests/bpf/prog_tests/tracing_struct.c
>> index ad46eabe784f8..e68fd5627ec1c 100644
>> --- a/tools/testing/selftests/bpf/prog_tests/tracing_struct.c
>> +++ b/tools/testing/selftests/bpf/prog_tests/tracing_struct.c
> [ ... ]
>> +/*
>> + * A static function whose BTF is unreliable gets a trampoline that only knows
>> + * of the register arguments, so one taking stack arguments must be refused.
>> + */
>> +static void test_tgt_prog_unreliable(void)
>> +{
> [ ... ]
>> +	err = bpf_program__set_attach_target(tracer->progs.unreliable_entry, fd,
>> +					     "callee_unreliable");
>> +	if (!ASSERT_OK(err, "set_attach_target"))
>> +		goto out;
>> +
>> +	err = tracing_struct_int128_tracer__load(tracer);
>> +	ASSERT_ERR(err, "tracer__load");
>> +	ASSERT_HAS_SUBSTR(log_buf, "unreliable BTF and stack arguments", "log");
> [Severity: High]
> While this test accurately checks callee_unreliable() which takes 6 scalar
> arguments, is there a potential issue in the verifier's
> check_unreliable_func_args() when dealing with struct arguments that occupy
> multiple slots?
>
> In kernel/bpf/verifier.c:check_unreliable_func_args(), if
> btf_distill_func_proto() fails (e.g. because an argument is a struct larger
> than 16 bytes), the fallback path counts the number of arguments instead
> of 8-byte slots:
>
> 	if (btf_distill_func_proto(NULL, btf, func_proto, tname, &m)) {
> 		/* at least a slot for each argument */
> 		nslots = btf_type_vlen(func_proto);
> 	}
>
> For a BPF subprogram taking a 32-byte struct by value and 4 other arguments
> (5 arguments total), the actual slot count is 1 + 1 + 1 + 1 + 4 = 8 slots
> (meaning 3 slots are on the stack). But the fallback calculates nslots = 5.

The compiler should already change 32-byte struct to a pointer to struct,
so the actual slot count should be 5. So we should be okay.

>
> Since 5 <= MAX_BPF_FUNC_REG_ARGS (5), check_unreliable_func_args()
> incorrectly returns 0, permitting attachment.
>
> The generated trampoline will not save or restore the stack arguments.
>
> Does this cause the target BPF subprogram to read uninitialized stack
> memory from the trampoline frame as its arguments, leading to data corruption
> and undefined behavior?
>

Re: [PATCH bpf-next v2 09/11] selftests/bpf: Test trampolines on more BPF functions with stack args

[Yonghong Song]

On 10/13/26 3:28 AM, bot+bpf-ci@kernel.org wrote:
>> selftests/bpf: Test trampolines on more BPF functions with stack args
> TL;DR: Nit only: the callee_split() comment in the selftest says h is split
> between register and stack, but that is x86-64 specific; on arm64 nothing is
> split. Suggests saying so.
>
>> diff --git a/tools/testing/selftests/bpf/progs/tracing_struct_int128_tgt.c b/tools/testing/selftests/bpf/progs/tracing_struct_int128_tgt.c
>> --- a/tools/testing/selftests/bpf/progs/tracing_struct_int128_tgt.c
>> +++ b/tools/testing/selftests/bpf/progs/tracing_struct_int128_tgt.c
> [ ... ]
>
>> @@ -44,6 +44,25 @@ static __noinline __u64 callee_stack(__u64 a, __u64 b, __u64 c, __u64 d,
>>   	       i * 10;
>>   }
>>
>> +/* BPF code splits h between the last argument register and the stack. */
>> +static __noinline __u64 callee_split(__u64 a, __u64 b, __u64 c, __u64 d,
>> +				     __u64 e, struct int128_arg h, __u64 i)
>> +{
>> +	return a + b * 2 + c * 3 + d * 4 + e * 5 + (__u64)h.v * 6 +
>> +	       (__u64)((unsigned __int128)h.v >> 64) * 7 + i * 8;
>> +}
> This isn't a bug, but is this comment accurate for every architecture this
> test runs on?
>
> test_int128_tgt_prog() is built for x86-64 and for arm64, and a..e already
> occupy R1-R5 in BPF terms, so h and i are each stack arguments as far as
> the BPF ISA is concerned.
>
> The split between a register and the stack only comes from the x86-64 JIT,
> which maps the first stack argument slot to r9:
>
> arch/x86/net/bpf_jit_comp.c:x86_arg_reg[] = { BPF_REG_1, ..., BPF_REG_5,
> X86_REG_R9 }
>
> On arm64 the first three stack argument slots are mapped to x5-x7, so h
> would land in x5/x6 and i in x7, with nothing split.
>
> The commit message already says "on x86-64 it is passed split between r9
> and the stack", and the comment on the neighbouring callee_stack() names
> x86-64 and arm64 separately.
>
> Could this comment say that the split is x86-64 specific, for example "x86-64
> BPF code passes h in r9 and its first stack slot"?

You are right. The comment is not good. Will fix.

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