Re: [PATCH bpf-next v9 4/7] libbpf: BPF programs manual loading and attaching

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] libbpf: bpf_map__init_kern_struct_ops() clobbers MANUAL load strategy
--

commit dc922049cb04bf1494a0c7805a4fd1d881505fad
Author: Slava Imameev <slava.imameev@crowdstrike.com>

libbpf: BPF programs manual loading and attaching

This commit adds support for BPF_PROG_LOAD_STRATEGY_MANUAL, allowing BPF
programs to be skipped during the initial bpf_object load and loaded or
unloaded on demand via bpf_program__load() and bpf_program__unload().

> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index e2cea1bca1d2d..1a5b582d63b24 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c

[ ... ]

> @@ -1227,6 +1228,14 @@ static int bpf_object_adjust_struct_ops_autoload(struct bpf_object *obj)
>  		if (prog->type != BPF_PROG_TYPE_STRUCT_OPS)
>  			continue;
>  
> +		/*
> +		 * If the user set the load strategy to MANUAL, don't guess it
> +		 * from map autocreate: a MANUAL program is loaded explicitly
> +		 * with bpf_program__load(), which rejects any other strategy.
> +		 */
> +		if (prog->load_strategy == BPF_PROG_LOAD_STRATEGY_MANUAL)
> +			continue;

[Severity: Medium]
Can bpf_map__init_kern_struct_ops() overwrite a struct_ops program's
BPF_PROG_LOAD_STRATEGY_MANUAL setting before this check in
bpf_object_adjust_struct_ops_autoload() is reached?

During bpf_object_prepare(), bpf_object__init_kern_struct_ops_maps() runs
immediately before bpf_object_adjust_struct_ops_autoload():

tools/lib/bpf/libbpf.c:bpf_object_prepare() {
    ...
    err = err ? : bpf_object__init_kern_struct_ops_maps(obj);
    err = err ? : bpf_object_adjust_struct_ops_autoload(obj);
    ...
}

Inside bpf_object__init_kern_struct_ops_maps() ->
bpf_map__init_kern_struct_ops(), if a callback member is absent in the
running kernel BTF (with a zeroed shadow field) or if its slot in a
struct_ops map's shadow struct was cleared or replaced, load_strategy is
unconditionally reset to BPF_PROG_LOAD_STRATEGY_DISABLED:

tools/lib/bpf/libbpf.c:bpf_map__init_kern_struct_ops() {
    ...
    if (!kern_member) {
        ...
        if (st_ops->progs[i]) {
            st_ops->progs[i]->load_strategy = BPF_PROG_LOAD_STRATEGY_DISABLED;
            st_ops->progs[i] = NULL;
        }
        ...
    }
    ...
    if (btf_is_ptr(mtype)) {
        prog = *(void **)mdata;
        if (st_ops->progs[i] && st_ops->progs[i] != prog)
            st_ops->progs[i]->load_strategy = BPF_PROG_LOAD_STRATEGY_DISABLED;
    ...
}

When a user sets BPF_PROG_LOAD_STRATEGY_MANUAL via
bpf_program__set_load_strategy(), those assignments clobber load_strategy to
BPF_PROG_LOAD_STRATEGY_DISABLED.

Then bpf_object_adjust_struct_ops_autoload() does not see
BPF_PROG_LOAD_STRATEGY_MANUAL, so it either leaves the program as
BPF_PROG_LOAD_STRATEGY_DISABLED (causing a subsequent bpf_program__load() to
fail with -EINVAL) or sets it to BPF_PROG_LOAD_STRATEGY_AUTO if another
autocreated struct_ops map still references it.

Should bpf_map__init_kern_struct_ops() preserve load_strategy when it is set
to BPF_PROG_LOAD_STRATEGY_MANUAL?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261029223348.3730738-1-andrey.grodzovsky@crowdstrike.com?part=4