Re: [PATCH bpf-next v3 3/4] libbpf: Move section-defined program flags to prog_flags

[bot+bpf-ci@kernel.org]

[-- Attachment #1: Type: text/plain, Size: 3218 bytes --]

> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index f09cbfd8e729d..c036e8a91ed88 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -7879,6 +7879,19 @@ static int tracing_multi_mod_fd(struct bpf_program *prog, int *btf_obj_fd)
>  	return 0;
>  }
>
> +static int libbpf_setup_prog_flags(struct bpf_program *prog, long cookie)
> +{
> +	enum sec_def_flags def = cookie;
> +
> +	if (def & SEC_SLEEPABLE)
> +		prog->prog_flags |= BPF_F_SLEEPABLE;
> +
> +	if (def & SEC_XDP_FRAGS)
> +		prog->prog_flags |= BPF_F_XDP_HAS_FRAGS;
> +
> +	return 0;
> +}
> +
>  /* this is called as prog->sec_def->prog_prepare_load_fn for libbpf-supported sec_defs */
>  static int libbpf_prepare_prog_load(struct bpf_program *prog,
>  				    struct bpf_prog_load_opts *opts, long cookie)
> @@ -7889,12 +7902,6 @@ static int libbpf_prepare_prog_load(struct bpf_program *prog,
>  	if ((def & SEC_EXP_ATTACH_OPT) && !kernel_supports(prog->obj, FEAT_EXP_ATTACH_TYPE))
>  		opts->expected_attach_type = 0;
>
> -	if (def & SEC_SLEEPABLE)
> -		opts->prog_flags |= BPF_F_SLEEPABLE;
> -
> -	if (prog->type == BPF_PROG_TYPE_XDP && (def & SEC_XDP_FRAGS))
> -		opts->prog_flags |= BPF_F_XDP_HAS_FRAGS;
> -

Does this change break bpf_program__clone() when the caller passes
non-zero opts.prog_flags?

Before this patch, libbpf_prepare_prog_load() OR'd in the section flags
after the caller's override:

    /* in bpf_program__clone() */
    attr.prog_flags = OPTS_GET(opts, prog_flags, 0) ?: prog->prog_flags;
    ...
    prog->sec_def->prog_prepare_load_fn(prog, &attr, ...);

    /* in libbpf_prepare_prog_load() */
    if (def & SEC_SLEEPABLE)
        opts->prog_flags |= BPF_F_SLEEPABLE;

So a caller doing:

    LIBBPF_OPTS(bpf_prog_load_opts, opts, .prog_flags = BPF_F_TEST_STATE_FREQ);
    fd = bpf_program__clone(prog, &opts);

would get TEST_STATE_FREQ | SLEEPABLE for a program in SEC("fentry.s/...").

After this patch, the flags live only in prog->prog_flags, and the `?:`
in bpf_program__clone() discards prog->prog_flags entirely when the
caller passes non-zero opts.prog_flags:

    attr.prog_flags = OPTS_GET(opts, prog_flags, 0) ?: prog->prog_flags;
                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
                      returns TEST_STATE_FREQ, prog->prog_flags is ignored

So the same caller now gets only TEST_STATE_FREQ without SLEEPABLE. For
sleepable-only helpers like bpf_copy_from_user, this produces a verifier
rejection. For BPF_PROG_TYPE_SYSCALL, which requires prog->sleepable,
it fails at load. For xdp.frags programs, BPF_F_XDP_HAS_FRAGS is lost.

Looking at the pattern immediately below in the same function:

    if (attr.token_fd)
        attr.prog_flags |= BPF_F_TOKEN_FD;

should the section flags be OR'd in unconditionally, or should the
caller-override `?:` be removed for prog_flags? The commit message
mentions bpf_program__set_flags() and bpf_program__set_type() as
affected APIs but doesn't mention bpf_program__clone().


---
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

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/33489985893