[PATCH bpf-next 1/2] libbpf: never interpret subprogs in .text as entry programs

[PATCH bpf-next 1/2] libbpf: never interpret subprogs in .text as entry programs

[Andrii Nakryiko]

Libbpf pre-1.0 had a legacy logic of allowing singular non-annotated
(i.e., not having explicit SEC() annotation) function to be treated as
sole entry BPF program (unless there were other explicit entry
programs).

This behavior was dropped during libbpf 1.0 transition period (unless
LIBBPF_STRICT_SEC_NAME flag was unset in libbpf_mode). When 1.0 was
released and all the legacy behavior was removed, the bug slipped
through leaving this legacy behavior around.

Fix this for good, as it actually causes very confusing behavior if BPF
object file only has subprograms, but no entry programs.

Fixes: bd054102a8c7 ("libbpf: enforce strict libbpf 1.0 behaviors")
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
 tools/lib/bpf/libbpf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 05ad264ff09b..7c40286c3948 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -4417,7 +4417,7 @@ static int bpf_object__collect_externs(struct bpf_object *obj)
 
 static bool prog_is_subprog(const struct bpf_object *obj, const struct bpf_program *prog)
 {
-	return prog->sec_idx == obj->efile.text_shndx && obj->nr_programs > 1;
+	return prog->sec_idx == obj->efile.text_shndx;
 }
 
 struct bpf_program *
-- 
2.43.5

[PATCH bpf-next 2/2] selftests/bpf: add subprog to BPF object file with no entry programs

[Andrii Nakryiko]

Add a subprogram to BPF object file that otherwise has no entry BPF
programs to validate that libbpf can still load this correctly.

Until this was fixed, user could expect this very confusing error message:

  libbpf: prog 'dangling_subprog': missing BPF prog type, check ELF section name '.text'
  libbpf: prog 'dangling_subprog': failed to load: -22
  libbpf: failed to load object 'struct_ops_detach'
  libbpf: failed to load BPF skeleton 'struct_ops_detach': -22

Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
 tools/testing/selftests/bpf/progs/struct_ops_detach.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/tools/testing/selftests/bpf/progs/struct_ops_detach.c b/tools/testing/selftests/bpf/progs/struct_ops_detach.c
index 56b787a89876..5222d58592a7 100644
--- a/tools/testing/selftests/bpf/progs/struct_ops_detach.c
+++ b/tools/testing/selftests/bpf/progs/struct_ops_detach.c
@@ -6,5 +6,11 @@
 
 char _license[] SEC("license") = "GPL";
 
+int dangling_subprog(void)
+{
+	/* do nothing, just be here */
+	return 0;
+}
+
 SEC(".struct_ops.link")
 struct bpf_testmod_ops testmod_do_detach;
-- 
2.43.5

Re: [PATCH bpf-next 2/2] selftests/bpf: add subprog to BPF object file with no entry programs

[Andrii Nakryiko]

On Thu, Oct 10, 2024 at 2:17 PM Andrii Nakryiko <andrii@kernel.org> wrote:
>
> Add a subprogram to BPF object file that otherwise has no entry BPF
> programs to validate that libbpf can still load this correctly.
>
> Until this was fixed, user could expect this very confusing error message:
>
>   libbpf: prog 'dangling_subprog': missing BPF prog type, check ELF section name '.text'
>   libbpf: prog 'dangling_subprog': failed to load: -22
>   libbpf: failed to load object 'struct_ops_detach'
>   libbpf: failed to load BPF skeleton 'struct_ops_detach': -22
>
> Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
> ---
>  tools/testing/selftests/bpf/progs/struct_ops_detach.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/tools/testing/selftests/bpf/progs/struct_ops_detach.c b/tools/testing/selftests/bpf/progs/struct_ops_detach.c
> index 56b787a89876..5222d58592a7 100644
> --- a/tools/testing/selftests/bpf/progs/struct_ops_detach.c
> +++ b/tools/testing/selftests/bpf/progs/struct_ops_detach.c
> @@ -6,5 +6,11 @@
>
>  char _license[] SEC("license") = "GPL";
>

argh, I had this comment here, which I missed to amend into this
patch, maybe whoever applies can just add it so I don't spam the
mailing list:

/* This subprogram validates that libbpf handles the situation in which BPF
 * object has subprograms in .text section, but has no entry BPF programs.
 * At some point that was causing issues due to legacy logic of treating such
 * subprogram as entry program (with unknown program type, which would fail).
 */
> +int dangling_subprog(void)
> +{
> +       /* do nothing, just be here */
> +       return 0;
> +}
> +
>  SEC(".struct_ops.link")
>  struct bpf_testmod_ops testmod_do_detach;
> --
> 2.43.5
>

Re: [PATCH bpf-next 1/2] libbpf: never interpret subprogs in .text as entry programs

[patchwork-bot+netdevbpf]

Hello:

This series was applied to bpf/bpf-next.git (master)
by Alexei Starovoitov <ast@kernel.org>:

On Thu, 10 Oct 2024 14:17:30 -0700 you wrote:
> Libbpf pre-1.0 had a legacy logic of allowing singular non-annotated
> (i.e., not having explicit SEC() annotation) function to be treated as
> sole entry BPF program (unless there were other explicit entry
> programs).
> 
> This behavior was dropped during libbpf 1.0 transition period (unless
> LIBBPF_STRICT_SEC_NAME flag was unset in libbpf_mode). When 1.0 was
> released and all the legacy behavior was removed, the bug slipped
> through leaving this legacy behavior around.
> 
> [...]

Here is the summary with links:
  - [bpf-next,1/2] libbpf: never interpret subprogs in .text as entry programs
    https://git.kernel.org/bpf/bpf-next/c/db089c9158c1
  - [bpf-next,2/2] selftests/bpf: add subprog to BPF object file with no entry programs
    https://git.kernel.org/bpf/bpf-next/c/82370ed5ade5

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html