Re: [PATCH bpf-next 1/2] bpf: Support fentry/fexit for functions with union args

[Leon Hwang <leon.hwang@linux.dev>]

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