Re: [PATCH bpf-next v2 3/5] libbpf: Convert bpf_program__set_autoload() to load-type enum

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] Modifying the signature of the public API `bpf_program__set_autoload` from `bool` to `enum bpf_prog_load_type` breaks ABI and C++ API compatibility.
- [Medium] Internal static function `bpf_program__set_dynamicload` incorrectly uses the public `libbpf_err()` wrapper, causing public APIs to omit it on their return paths.
--

commit 21a83792a849333b99398b408870c76df6690254
Author: Andrey Grodzovsky <andrey.grodzovsky@crowdstrike.com>

libbpf: Convert bpf_program__set_autoload() to load-type enum

This commit converts bpf_program__set_autoload() from bool to enum
bpf_prog_load_type to unify the API. It also restores autoattach when
leaving the dynamic load type.

> diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
> --- a/tools/lib/bpf/libbpf.h
> +++ b/tools/lib/bpf/libbpf.h
[ ... ]
> @@ -376,7 +389,18 @@ LIBBPF_API void bpf_program__set_ifindex(struct bpf_program *prog,
>  LIBBPF_API const char *bpf_program__name(const struct bpf_program *prog);
>  LIBBPF_API const char *bpf_program__section_name(const struct bpf_program *prog);
>  LIBBPF_API bool bpf_program__autoload(const struct bpf_program *prog);
> -LIBBPF_API int bpf_program__set_autoload(struct bpf_program *prog, bool autoload);
> +/**
> + * @brief **bpf_program__set_autoload()** is retained for backwards
> + * compatibility; **bpf_program__set_load_type()** is the preferred API.
> + * The enum's numbering (DISABLED=0, AUTO=1, DYNAMIC=2) matches the previous
> + * bool semantics (false=0, true=1), so existing bool callers keep compiling
> + * and behaving unchanged. Only BPF_PROG_LOAD_TYPE_AUTO and
> + * BPF_PROG_LOAD_TYPE_DISABLED are accepted, preserving the original
> + * autoload on/off meaning; use bpf_program__set_load_type() to set
> + * BPF_PROG_LOAD_TYPE_DYNAMIC.
> + */
> +LIBBPF_API int bpf_program__set_autoload(struct bpf_program *prog,
> +					 enum bpf_prog_load_type autoload);

[Severity: Critical]
Does this signature change introduce a regression for existing compiled
binaries and C++ consumers?

Existing binaries compiled with the old bool signature pass a 1-byte value
in a register. The new library expects a 32-bit enum, which might read
undefined garbage from the upper 24 bits of the lower 32-bit register half.

This garbage could cause the validation in bpf_program__set_autoload() to
falsely trigger and return -EINVAL:

tools/lib/bpf/libbpf.c:bpf_program__set_autoload() {
    if (autoload != BPF_PROG_LOAD_TYPE_AUTO &&
        autoload != BPF_PROG_LOAD_TYPE_DISABLED)
            return libbpf_err(-EINVAL);
    ...
}

Furthermore, C++ code calling bpf_program__set_autoload(prog, true) will
fail to compile because C++ does not allow implicit conversion from bool
to an unscoped enum type.

> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
[ ... ]
> @@ -15295,15 +15289,28 @@ static int bpf_program__set_dynamicload(struct bpf_program *prog)
>  
>  int bpf_program__set_load_type(struct bpf_program *prog, enum bpf_prog_load_type type)
>  {
> +	if (!prog)
> +		return libbpf_err(-EINVAL);
> +
>  	if (prog->obj->state >= OBJ_LOADED)
>  		return libbpf_err(-EINVAL);
>  
>  	switch (type) {
>  	case BPF_PROG_LOAD_TYPE_DYNAMIC:
>  		return bpf_program__set_dynamicload(prog);

[Severity: Medium]
Does this propagate an already-wrapped error code rather than applying the
wrapper at the public boundary?

The libbpf error handling convention mandates that errno must be set exactly
at the public API boundary, and internal functions like
bpf_program__set_dynamicload() must return raw negative error codes.

Because bpf_program__set_dynamicload() incorrectly calls libbpf_err()
internally, public APIs like bpf_program__set_load_type() and
bpf_program__set_autoload() end up propagating this wrapped error code
directly without applying the wrapper on their own return paths.

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