Skip to content

Conversation

@kernel-patches-daemon-bpf-rc
Copy link

Pull request for series with
subject: Support associating BPF programs with struct_ops
version: 2
url: https://patchwork.kernel.org/project/netdevbpf/list/?series=1012542

@kernel-patches-daemon-bpf-rc
Copy link
Author

Upstream branch: 48a97ff
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1012542
version: 2

@kernel-patches-review-bot
Copy link

commit 13cd013ea962cc2913a1840ef05e1a0e64371d6f
Author: Amery Hung <[email protected]>

bpf: Allow verifier to fixup kernel module kfuncs

Allow verifier to fixup kfuncs in kernel module to support kfuncs with
__prog arguments. Currently, special kfuncs and kfuncs with __prog
arguments are kernel kfuncs. Allowing kernel module kfuncs should not
affect existing kfunc fixup as kernel module kfuncs have BTF IDs greater
than kernel kfuncs' BTF IDs.

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index c908015b2..b8387dedd 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -21917,8 +21917,7 @@ static int fixup_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
>
>  	if (!bpf_jit_supports_far_kfunc_call())
>  		insn->imm = BPF_CALL_IMM(desc->addr);
> -	if (insn->off)
> -		return 0;
> +

Can module kfuncs incorrectly match special kfunc BTF IDs?

The removed check prevented module kfuncs (insn->off != 0) from reaching
the special kfunc handling code below. Now that module kfuncs continue
through this code, fixup_kfunc_call() compares desc->func_id against
special_kfunc_list[] entries without checking desc->offset.

BTF IDs are namespace-local: a module's BTF ID 42 is independent from
the kernel's BTF ID 42. When find_kfunc_desc() at line 21911 returns a
descriptor using (func_id, offset) as the key, desc->func_id contains
the raw BTF ID from whichever BTF namespace the kfunc belongs to.

The comparisons starting at line 21921:

    if (desc->func_id == special_kfunc_list[KF_bpf_obj_new_impl] || ...)

compare a potentially module-local BTF ID against kernel BTF IDs from
special_kfunc_list[], without verifying desc->offset == 0. If a module
happens to export a kfunc whose BTF ID numerically equals one of the ~50
special kfunc IDs, the verifier would incorrectly apply special handling
meant for kernel-only operations like bpf_obj_new_impl or
bpf_list_push_front_impl.

This could cause the verifier to:
- Access env->insn_aux_data[insn_idx].kptr_struct_meta which wouldn't be
  set for regular module kfuncs
- Trigger verifier_bug() at lines 21928, 21945, 21952, or 21975 due to
  unexpected NULL/non-NULL kptr_struct_meta
- Generate incorrect instruction fixups for the module kfunc

With kernel BTF containing thousands of types and special_kfunc_list[]
spanning ~50 function IDs, collisions between module function BTF IDs
and special kfunc IDs seem likely across different kernel modules.

Should the special kfunc checks include an offset validation, such as:

    if (desc->offset == 0 &&
        (desc->func_id == special_kfunc_list[KF_bpf_obj_new_impl] || ...))

to ensure only kernel kfuncs can match special kfunc entries?

>  	if (desc->func_id == special_kfunc_list[KF_bpf_obj_new_impl] ||
>  	    desc->func_id == special_kfunc_list[KF_bpf_percpu_obj_new_impl]) {
>  		struct btf_struct_meta *kptr_struct_meta = env->insn_aux_data[insn_idx].kptr_struct_meta;



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

In-Reply-To-Subject: bpf: Allow verifier to fixup kernel module kfuncs
CI run summary: https://github.com/kernel-patches/bpf-rc/actions/runs/18574367478

@kernel-patches-daemon-bpf-rc
Copy link
Author

Forwarding comment 3412832984 via email
In-Reply-To: [email protected]
Patch: https://patchwork.kernel.org/project/netdevbpf/patch/[email protected]/

@kernel-patches-daemon-bpf-rc
Copy link
Author

Upstream branch: 48a97ff
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1012542
version: 2

Allow verifier to fixup kfuncs in kernel module to support kfuncs with
__prog arguments. Currently, special kfuncs and kfuncs with __prog
arguments are kernel kfuncs. Allowing kernel module kfuncs should not
affect existing kfunc fixup as kernel module kfuncs have BTF IDs greater
than kernel kfuncs' BTF IDs.

Signed-off-by: Amery Hung <[email protected]>
Add a new BPF command BPF_PROG_ASSOC_STRUCT_OPS to allow associating
a BPF program with a struct_ops map. This command takes a file
descriptor of a struct_ops map and a BPF program and set
prog->aux->st_ops_assoc to the kdata of the struct_ops map.

The command does not accept a struct_ops program or a non-struct_ops
map. Programs of a struct_ops map is automatically associated with the
map during map update. If a program is shared between two struct_ops
maps, prog->aux->st_ops_assoc will be poisoned to indicate that the
associated struct_ops is ambiguous. A kernel helper
bpf_prog_get_assoc_struct_ops() can be used to retrieve the pointer.
The associated struct_ops map, once set, cannot be changed later. This
restriction may be lifted in the future if there is a use case.

Each associated programs except struct_ops programs of the map will take
a refcount on the map to pin it so that prog->aux->st_ops_assoc, if set,
is always valid. However, it is not guaranteed whether the map members
are fully updated nor is it attached or not. For example, a BPF program
can be associated with a struct_ops map before map_update. The
struct_ops implementer will be responsible for maintaining and checking
the state of the associated struct_ops map before accessing it.

Signed-off-by: Amery Hung <[email protected]>
Add low-level wrapper API for BPF_PROG_ASSOC_STRUCT_OPS command in the
bpf() syscall.

Signed-off-by: Amery Hung <[email protected]>
Test BPF_PROG_ASSOC_STRUCT_OPS command that associates a BPF program
with a struct_ops. The test follows the same logic in commit
ba7000f ("selftests/bpf: Test multi_st_ops and calling kfuncs from
different programs"), but instead of using map id to identify a specific
struct_ops, this test uses the new BPF command to associate a struct_ops
with a program.

The test consists of two sets of almost identical struct_ops maps and BPF
programs associated with the map. Their only difference is the unique
value returned by bpf_testmod_multi_st_ops::test_1().

The test first loads the programs and associates them with struct_ops
maps. Then, it exercises the BPF programs. They will in turn call kfunc
bpf_kfunc_multi_st_ops_test_1_prog_arg() to trigger test_1() of the
associated struct_ops map, and then check if the right unique value is
returned.

Signed-off-by: Amery Hung <[email protected]>
@kernel-patches-daemon-bpf-rc
Copy link
Author

Upstream branch: 48a97ff
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1012542
version: 2

@kernel-patches-daemon-bpf-rc
Copy link
Author

At least one diff in series https://patchwork.kernel.org/project/netdevbpf/list/?series=1012542 expired. Closing PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants