BPF List
 help / color / mirror / Atom feed
* [PATCH bpf-next 0/2] bpf: Support fentry/fexit for functions with union args
@ 2025-09-05 13:32 Leon Hwang
  2025-09-05 13:32 ` [PATCH bpf-next 1/2] " Leon Hwang
  2025-09-05 13:32 ` [PATCH bpf-next 2/2] selftests/bpf: Add test to access union argument in tracing program Leon Hwang
  0 siblings, 2 replies; 5+ messages in thread
From: Leon Hwang @ 2025-09-05 13:32 UTC (permalink / raw)
  To: bpf
  Cc: ast, andrii, daniel, martin.lau, eddyz87, song, yonghong.song,
	leon.hwang, kernel-patches-bot

While tracing 'release_pages' with bpfsnoop[0], the verifier reports:

The function release_pages arg0 type UNION is unsupported.

However, it should be acceptable to trace functions that have 'union'
arguments.

This patch set enables such support in the verifier by allowing 'union'
as a valid argument type.

Links:
[0] https://github.com/bpfsnoop/bpfsnoop

Leon Hwang (2):
  bpf: Allow tracing union-arg functions
  selftests/bpf: Add test to access union argument in tracing program

 kernel/bpf/btf.c                                   |  4 ++--
 net/bpf/test_run.c                                 | 14 +++++++++++++-
 .../selftests/bpf/progs/verifier_btf_ctx_access.c  | 12 ++++++++++++
 3 files changed, 27 insertions(+), 3 deletions(-)

--
2.50.1


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH bpf-next 1/2] bpf: Support fentry/fexit for functions with union args
  2025-09-05 13:32 [PATCH bpf-next 0/2] bpf: Support fentry/fexit for functions with union args Leon Hwang
@ 2025-09-05 13:32 ` Leon Hwang
  2025-09-10  0:54   ` Alexei Starovoitov
  2025-09-05 13:32 ` [PATCH bpf-next 2/2] selftests/bpf: Add test to access union argument in tracing program Leon Hwang
  1 sibling, 1 reply; 5+ messages in thread
From: Leon Hwang @ 2025-09-05 13:32 UTC (permalink / raw)
  To: bpf
  Cc: ast, andrii, daniel, martin.lau, eddyz87, song, yonghong.song,
	leon.hwang, kernel-patches-bot

Currently, functions with 'union' arguments cannot be traced with
fentry/fexit:

bpftrace -e 'fentry:release_pages { exit(); }' -v
AST node count: 6
Attaching 1 probe...
ERROR: Error loading BPF program for fentry_vmlinux_release_pages_1.
Kernel error log:
The function release_pages arg0 type UNION is unsupported.
processed 0 insns (limit 1000000) max_states_per_insn 0 total_states 0 peak_states 0 mark_read 0

ERROR: Loading BPF object(s) failed.

The type of the 'release_pages' argument is defined as:

typedef union {
	struct page **pages;
	struct folio **folios;
	struct encoded_page **encoded_pages;
} release_pages_arg __attribute__ ((__transparent_union__));

This patch relaxes the restriction by allowing function arguments of type
'union' to be traced.

Signed-off-by: Leon Hwang <leon.hwang@linux.dev>
---
 kernel/bpf/btf.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index 64739308902f7..86883b3c97d20 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -6762,7 +6762,7 @@ bool btf_ctx_access(int off, int size, enum bpf_access_type type,
 	/* skip modifiers */
 	while (btf_type_is_modifier(t))
 		t = btf_type_by_id(btf, t->type);
-	if (btf_type_is_small_int(t) || btf_is_any_enum(t) || __btf_type_is_struct(t))
+	if (btf_type_is_small_int(t) || btf_is_any_enum(t) || btf_type_is_struct(t))
 		/* accessing a scalar */
 		return true;
 	if (!btf_type_is_ptr(t)) {
@@ -7334,7 +7334,7 @@ static int __get_type_size(struct btf *btf, u32 btf_id,
 	if (btf_type_is_ptr(t))
 		/* kernel size of pointer. Not BPF's size of pointer*/
 		return sizeof(void *);
-	if (btf_type_is_int(t) || btf_is_any_enum(t) || __btf_type_is_struct(t))
+	if (btf_type_is_int(t) || btf_is_any_enum(t) || btf_type_is_struct(t))
 		return t->size;
 	return -EINVAL;
 }
--
2.50.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH bpf-next 2/2] selftests/bpf: Add test to access union argument in tracing program
  2025-09-05 13:32 [PATCH bpf-next 0/2] bpf: Support fentry/fexit for functions with union args Leon Hwang
  2025-09-05 13:32 ` [PATCH bpf-next 1/2] " Leon Hwang
@ 2025-09-05 13:32 ` Leon Hwang
  1 sibling, 0 replies; 5+ messages in thread
From: Leon Hwang @ 2025-09-05 13:32 UTC (permalink / raw)
  To: bpf
  Cc: ast, andrii, daniel, martin.lau, eddyz87, song, yonghong.song,
	leon.hwang, kernel-patches-bot

Adding verifier test for accessing union argument in tracing programs.

The test program loads 1st argument of bpf_fentry_test11 function
which is union and checks that verifier allows that.

cd tools/testing/selftests/bpf
./test_progs -t verifier_btf_ctx
501/7   verifier_btf_ctx_access/btf_ctx_access union arg accept:OK
501     verifier_btf_ctx_access:OK
Summary: 1/7 PASSED, 0 SKIPPED, 0 FAILED

Signed-off-by: Leon Hwang <leon.hwang@linux.dev>
---
 net/bpf/test_run.c                                 | 14 +++++++++++++-
 .../selftests/bpf/progs/verifier_btf_ctx_access.c  | 12 ++++++++++++
 2 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
index 4a862d6053861..c65d468fd6012 100644
--- a/net/bpf/test_run.c
+++ b/net/bpf/test_run.c
@@ -574,6 +574,16 @@ noinline int bpf_fentry_test10(const void *a)
 	return (long)a;
 }
 
+typedef union {
+	void *arg0;
+	int *arg1;
+} union_test_t;
+
+noinline int bpf_fentry_test11(union_test_t t)
+{
+	return (int)(long)t.arg0;
+}
+
 noinline void bpf_fentry_test_sinfo(struct skb_shared_info *sinfo)
 {
 }
@@ -688,6 +698,7 @@ int bpf_prog_test_run_tracing(struct bpf_prog *prog,
 	struct bpf_fentry_test_t arg = {};
 	u16 side_effect = 0, ret = 0;
 	int b = 2, err = -EFAULT;
+	union_test_t utt = {};
 	u32 retval = 0;
 
 	if (kattr->test.flags || kattr->test.cpu || kattr->test.batch_size)
@@ -705,7 +716,8 @@ int bpf_prog_test_run_tracing(struct bpf_prog *prog,
 		    bpf_fentry_test7((struct bpf_fentry_test_t *)0) != 0 ||
 		    bpf_fentry_test8(&arg) != 0 ||
 		    bpf_fentry_test9(&retval) != 0 ||
-		    bpf_fentry_test10((void *)0) != 0)
+		    bpf_fentry_test10((void *)0) != 0 ||
+		    bpf_fentry_test11(utt) != 0)
 			goto out;
 		break;
 	case BPF_MODIFY_RETURN:
diff --git a/tools/testing/selftests/bpf/progs/verifier_btf_ctx_access.c b/tools/testing/selftests/bpf/progs/verifier_btf_ctx_access.c
index 03942cec07e56..ff379836b5f00 100644
--- a/tools/testing/selftests/bpf/progs/verifier_btf_ctx_access.c
+++ b/tools/testing/selftests/bpf/progs/verifier_btf_ctx_access.c
@@ -77,4 +77,16 @@ __naked void ctx_access_const_void_pointer_accept(void)
 "	::: __clobber_all);
 }
 
+SEC("fentry/bpf_fentry_test11")
+__description("btf_ctx_access union arg accept")
+__success __retval(0)
+__naked void ctx_access_union_arg_accept(void)
+{
+	asm volatile ("					\
+	r2 = *(u64 *)(r1 + 0);		/* load 1st argument value (union) */\
+	r0 = 0;						\
+	exit;						\
+"	::: __clobber_all);
+}
+
 char _license[] SEC("license") = "GPL";
-- 
2.50.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH bpf-next 1/2] bpf: Support fentry/fexit for functions with union args
  2025-09-05 13:32 ` [PATCH bpf-next 1/2] " Leon Hwang
@ 2025-09-10  0:54   ` Alexei Starovoitov
  2025-09-12  2:52     ` Leon Hwang
  0 siblings, 1 reply; 5+ messages in thread
From: Alexei Starovoitov @ 2025-09-10  0:54 UTC (permalink / raw)
  To: Leon Hwang
  Cc: bpf, Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
	Martin KaFai Lau, Eduard, Song Liu, Yonghong Song,
	kernel-patches-bot

On Fri, Sep 5, 2025 at 6:32 AM Leon Hwang <leon.hwang@linux.dev> wrote:
>
> Currently, functions with 'union' arguments cannot be traced with
> fentry/fexit:

union-s passed _by value_.
It's an important detail.

>
> bpftrace -e 'fentry:release_pages { exit(); }' -v
> AST node count: 6
> Attaching 1 probe...
> ERROR: Error loading BPF program for fentry_vmlinux_release_pages_1.
> Kernel error log:
> The function release_pages arg0 type UNION is unsupported.
> processed 0 insns (limit 1000000) max_states_per_insn 0 total_states 0 peak_states 0 mark_read 0
>
> ERROR: Loading BPF object(s) failed.
>
> The type of the 'release_pages' argument is defined as:
>
> typedef union {
>         struct page **pages;
>         struct folio **folios;
>         struct encoded_page **encoded_pages;
> } release_pages_arg __attribute__ ((__transparent_union__));
>
> This patch relaxes the restriction by allowing function arguments of type
> 'union' to be traced.
>
> Signed-off-by: Leon Hwang <leon.hwang@linux.dev>
> ---
>  kernel/bpf/btf.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> index 64739308902f7..86883b3c97d20 100644
> --- a/kernel/bpf/btf.c
> +++ b/kernel/bpf/btf.c
> @@ -6762,7 +6762,7 @@ bool btf_ctx_access(int off, int size, enum bpf_access_type type,
>         /* skip modifiers */
>         while (btf_type_is_modifier(t))
>                 t = btf_type_by_id(btf, t->type);
> -       if (btf_type_is_small_int(t) || btf_is_any_enum(t) || __btf_type_is_struct(t))
> +       if (btf_type_is_small_int(t) || btf_is_any_enum(t) || btf_type_is_struct(t))
>                 /* accessing a scalar */
>                 return true;
>         if (!btf_type_is_ptr(t)) {
> @@ -7334,7 +7334,7 @@ static int __get_type_size(struct btf *btf, u32 btf_id,
>         if (btf_type_is_ptr(t))
>                 /* kernel size of pointer. Not BPF's size of pointer*/
>                 return sizeof(void *);
> -       if (btf_type_is_int(t) || btf_is_any_enum(t) || __btf_type_is_struct(t))
> +       if (btf_type_is_int(t) || btf_is_any_enum(t) || btf_type_is_struct(t))
>                 return t->size;

Did you look at
commit 720e6a435194 ("bpf: Allow struct argument in trampoline based programs")
that added support for accessing struct passed by value?

Study it and figure out what part of the verifier you forgot
to update while adding this support for accessing unions
passed by value.
Think it through and update the selftest to make sure it tests
the support end-to-end and covers the bug in this patch.

pw-bot: cr

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH bpf-next 1/2] bpf: Support fentry/fexit for functions with union args
  2025-09-10  0:54   ` Alexei Starovoitov
@ 2025-09-12  2:52     ` Leon Hwang
  0 siblings, 0 replies; 5+ messages in thread
From: Leon Hwang @ 2025-09-12  2:52 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: bpf, Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
	Martin KaFai Lau, Eduard, Song Liu, Yonghong Song,
	kernel-patches-bot



On 10/9/25 08:54, Alexei Starovoitov wrote:
> On Fri, Sep 5, 2025 at 6:32 AM Leon Hwang <leon.hwang@linux.dev> wrote:
>>
>> Currently, functions with 'union' arguments cannot be traced with
>> fentry/fexit:
> 
> union-s passed _by value_.
> It's an important detail.
> 
>>
>> bpftrace -e 'fentry:release_pages { exit(); }' -v
>> AST node count: 6
>> Attaching 1 probe...
>> ERROR: Error loading BPF program for fentry_vmlinux_release_pages_1.
>> Kernel error log:
>> The function release_pages arg0 type UNION is unsupported.
>> processed 0 insns (limit 1000000) max_states_per_insn 0 total_states 0 peak_states 0 mark_read 0
>>
>> ERROR: Loading BPF object(s) failed.
>>
>> The type of the 'release_pages' argument is defined as:
>>
>> typedef union {
>>         struct page **pages;
>>         struct folio **folios;
>>         struct encoded_page **encoded_pages;
>> } release_pages_arg __attribute__ ((__transparent_union__));
>>
>> This patch relaxes the restriction by allowing function arguments of type
>> 'union' to be traced.
>>
>> Signed-off-by: Leon Hwang <leon.hwang@linux.dev>
>> ---
>>  kernel/bpf/btf.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
>> index 64739308902f7..86883b3c97d20 100644
>> --- a/kernel/bpf/btf.c
>> +++ b/kernel/bpf/btf.c
>> @@ -6762,7 +6762,7 @@ bool btf_ctx_access(int off, int size, enum bpf_access_type type,
>>         /* skip modifiers */
>>         while (btf_type_is_modifier(t))
>>                 t = btf_type_by_id(btf, t->type);
>> -       if (btf_type_is_small_int(t) || btf_is_any_enum(t) || __btf_type_is_struct(t))
>> +       if (btf_type_is_small_int(t) || btf_is_any_enum(t) || btf_type_is_struct(t))
>>                 /* accessing a scalar */
>>                 return true;
>>         if (!btf_type_is_ptr(t)) {
>> @@ -7334,7 +7334,7 @@ static int __get_type_size(struct btf *btf, u32 btf_id,
>>         if (btf_type_is_ptr(t))
>>                 /* kernel size of pointer. Not BPF's size of pointer*/
>>                 return sizeof(void *);
>> -       if (btf_type_is_int(t) || btf_is_any_enum(t) || __btf_type_is_struct(t))
>> +       if (btf_type_is_int(t) || btf_is_any_enum(t) || btf_type_is_struct(t))
>>                 return t->size;
> 
> Did you look at
> commit 720e6a435194 ("bpf: Allow struct argument in trampoline based programs")
> that added support for accessing struct passed by value?
> 
> Study it and figure out what part of the verifier you forgot
> to update while adding this support for accessing unions
> passed by value.
> Think it through and update the selftest to make sure it tests
> the support end-to-end and covers the bug in this patch.
> 

Thanks for pointing this out.

You’re right — support for union arguments should follow the same
approach used for struct arguments in commit
720e6a435194 ("bpf: Allow struct argument in trampoline based
programs"). My current patch was too naive and missed several necessary
updates.

I’ll review that commit carefully, identify what I overlooked, and align
the implementation for unions accordingly.

I’ll also extend the selftests to ensure we have end-to-end coverage and
can catch the kind of bug exposed here.

Thanks,
Leon


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2025-09-12  2:52 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-09-05 13:32 [PATCH bpf-next 0/2] bpf: Support fentry/fexit for functions with union args Leon Hwang
2025-09-05 13:32 ` [PATCH bpf-next 1/2] " Leon Hwang
2025-09-10  0:54   ` Alexei Starovoitov
2025-09-12  2:52     ` Leon Hwang
2025-09-05 13:32 ` [PATCH bpf-next 2/2] selftests/bpf: Add test to access union argument in tracing program Leon Hwang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox