[PATCH bpf-next 0/3] Fix handling of CO-RE relos for __weak subprogs

[PATCH bpf-next 0/3] Fix handling of CO-RE relos for __weak subprogs

[Andrii Nakryiko]

Fix the issue accidentally discovered during libbpf USDT support testing.
Libbpf overzealously complained about CO-RE relocations belonging to the code
of a __weak subprog that got overriden by another instance of that function.

Fix the issue fixed, return back to __weak __hidden annotation for USDT
BPF-side APIs.

And add CO-RE relos to linked_funcs selftest to ensure such combo keeps
working going forward.

Andrii Nakryiko (3):
  libbpf: don't error out on CO-RE relos for overriden weak subprogs
  libbpf: use weak hidden modifier for USDT BPF-side API functions
  selftests/bpf: add CO-RE relos into linked_funcs selftests

 tools/lib/bpf/libbpf.c                            | 15 +++++++++++----
 tools/lib/bpf/usdt.bpf.h                          |  6 +++---
 tools/testing/selftests/bpf/progs/linked_funcs1.c |  8 ++++++++
 tools/testing/selftests/bpf/progs/linked_funcs2.c |  8 ++++++++
 4 files changed, 30 insertions(+), 7 deletions(-)

-- 
2.30.2

[PATCH bpf-next 1/3] libbpf: don't error out on CO-RE relos for overriden weak subprogs

[Andrii Nakryiko]

During BPF static linking, all the ELF relocations and .BTF.ext
information (including CO-RE relocations) are preserved for __weak
subprograms that were logically overriden by either previous weak
subprogram instance or by corresponding "strong" (non-weak) subprogram.
This is just how native user-space linkers work, nothing new.

But libbpf is over-zealous when processing CO-RE relocation to error out
when CO-RE relocation belonging to such eliminated weak subprogram is
encountered. Instead of erroring out on this expected situation, log
debug-level message and skip the relocation.

Fixes: db2b8b06423c ("libbpf: Support CO-RE relocations for multi-prog sections")
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
 tools/lib/bpf/libbpf.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 9deb1fc67f19..465b7c0996f1 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -5687,10 +5687,17 @@ bpf_object__relocate_core(struct bpf_object *obj, const char *targ_btf_path)
 			insn_idx = rec->insn_off / BPF_INSN_SZ;
 			prog = find_prog_by_sec_insn(obj, sec_idx, insn_idx);
 			if (!prog) {
-				pr_warn("sec '%s': failed to find program at insn #%d for CO-RE offset relocation #%d\n",
-					sec_name, insn_idx, i);
-				err = -EINVAL;
-				goto out;
+				/* When __weak subprog is "overridden" by another instance
+				 * of the subprog from a different object file, linker still
+				 * appends all the .BTF.ext info that used to belong to that
+				 * eliminated subprogram.
+				 * This is similar to what x86-64 linker does for relocations.
+				 * So just ignore such relocations just like we ignore
+				 * subprog instructions when discovering subprograms.
+				 */
+				pr_debug("sec '%s': skipping CO-RE relocation #%d for insn #%d belonging to eliminated weak subprogram\n",
+					 sec_name, i, insn_idx);
+				continue;
 			}
 			/* no need to apply CO-RE relocation if the program is
 			 * not going to be loaded
-- 
2.30.2

[PATCH bpf-next 2/3] libbpf: use weak hidden modifier for USDT BPF-side API functions

[Andrii Nakryiko]

Use __weak __hidden for bpf_usdt_xxx() APIs instead of much more
confusing `static inline __noinline`. This was previously impossible due
to libbpf erroring out on CO-RE relocations pointing to eliminated weak
subprogs. Now that previous patch fixed this issue, switch back to
__weak __hidden as it's a more direct way of specifying the desired
behavior.

Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
 tools/lib/bpf/usdt.bpf.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/lib/bpf/usdt.bpf.h b/tools/lib/bpf/usdt.bpf.h
index 881a2422a8ef..4181fddb3687 100644
--- a/tools/lib/bpf/usdt.bpf.h
+++ b/tools/lib/bpf/usdt.bpf.h
@@ -103,7 +103,7 @@ int __bpf_usdt_spec_id(struct pt_regs *ctx)
 }
 
 /* Return number of USDT arguments defined for currently traced USDT. */
-static inline __noinline
+__weak __hidden
 int bpf_usdt_arg_cnt(struct pt_regs *ctx)
 {
 	struct __bpf_usdt_spec *spec;
@@ -124,7 +124,7 @@ int bpf_usdt_arg_cnt(struct pt_regs *ctx)
  * Returns 0 on success; negative error, otherwise.
  * On error *res is guaranteed to be set to zero.
  */
-static inline __noinline
+__weak __hidden
 int bpf_usdt_arg(struct pt_regs *ctx, __u64 arg_num, long *res)
 {
 	struct __bpf_usdt_spec *spec;
@@ -204,7 +204,7 @@ int bpf_usdt_arg(struct pt_regs *ctx, __u64 arg_num, long *res)
  * utilizing BPF cookies internally, so user can't use BPF cookie directly
  * for USDT programs and has to use bpf_usdt_cookie() API instead.
  */
-static inline __noinline
+__weak __hidden
 long bpf_usdt_cookie(struct pt_regs *ctx)
 {
 	struct __bpf_usdt_spec *spec;
-- 
2.30.2

[PATCH bpf-next 3/3] selftests/bpf: add CO-RE relos into linked_funcs selftests

[Andrii Nakryiko]

Add CO-RE relocations into __weak subprogs for multi-file linked_funcs
selftest to make sure libbpf handles such combination well.

Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
 tools/testing/selftests/bpf/progs/linked_funcs1.c | 8 ++++++++
 tools/testing/selftests/bpf/progs/linked_funcs2.c | 8 ++++++++
 2 files changed, 16 insertions(+)

diff --git a/tools/testing/selftests/bpf/progs/linked_funcs1.c b/tools/testing/selftests/bpf/progs/linked_funcs1.c
index b964ec1390c2..963b393c37e8 100644
--- a/tools/testing/selftests/bpf/progs/linked_funcs1.c
+++ b/tools/testing/selftests/bpf/progs/linked_funcs1.c
@@ -4,6 +4,7 @@
 #include "vmlinux.h"
 #include <bpf/bpf_helpers.h>
 #include <bpf/bpf_tracing.h>
+#include <bpf/bpf_core_read.h>
 
 /* weak and shared between two files */
 const volatile int my_tid __weak;
@@ -44,6 +45,13 @@ void set_output_ctx1(__u64 *ctx)
 /* this weak instance should win because it's the first one */
 __weak int set_output_weak(int x)
 {
+	static volatile int whatever;
+
+	/* make sure we use CO-RE relocations in a weak function, this used to
+	 * cause problems for BPF static linker
+	 */
+	whatever = bpf_core_type_size(struct task_struct);
+
 	output_weak1 = x;
 	return x;
 }
diff --git a/tools/testing/selftests/bpf/progs/linked_funcs2.c b/tools/testing/selftests/bpf/progs/linked_funcs2.c
index 575e958e60b7..db195872f4eb 100644
--- a/tools/testing/selftests/bpf/progs/linked_funcs2.c
+++ b/tools/testing/selftests/bpf/progs/linked_funcs2.c
@@ -4,6 +4,7 @@
 #include "vmlinux.h"
 #include <bpf/bpf_helpers.h>
 #include <bpf/bpf_tracing.h>
+#include <bpf/bpf_core_read.h>
 
 /* weak and shared between both files */
 const volatile int my_tid __weak;
@@ -44,6 +45,13 @@ void set_output_ctx2(__u64 *ctx)
 /* this weak instance should lose, because it will be processed second */
 __weak int set_output_weak(int x)
 {
+	static volatile int whatever;
+
+	/* make sure we use CO-RE relocations in a weak function, this used to
+	 * cause problems for BPF static linker
+	 */
+	whatever = 2 * bpf_core_type_size(struct task_struct);
+
 	output_weak2 = x;
 	return 2 * x;
 }
-- 
2.30.2

Re: [PATCH bpf-next 0/3] Fix handling of CO-RE relos for __weak subprogs

[Andrii Nakryiko]

On Fri, Apr 8, 2022 at 11:14 AM Andrii Nakryiko <andrii@kernel.org> wrote:
>
> Fix the issue accidentally discovered during libbpf USDT support testing.
> Libbpf overzealously complained about CO-RE relocations belonging to the code
> of a __weak subprog that got overriden by another instance of that function.
>
> Fix the issue fixed, return back to __weak __hidden annotation for USDT

"With the issue fixed", sigh...

> BPF-side APIs.
>
> And add CO-RE relos to linked_funcs selftest to ensure such combo keeps
> working going forward.
>
> Andrii Nakryiko (3):
>   libbpf: don't error out on CO-RE relos for overriden weak subprogs
>   libbpf: use weak hidden modifier for USDT BPF-side API functions
>   selftests/bpf: add CO-RE relos into linked_funcs selftests
>
>  tools/lib/bpf/libbpf.c                            | 15 +++++++++++----
>  tools/lib/bpf/usdt.bpf.h                          |  6 +++---
>  tools/testing/selftests/bpf/progs/linked_funcs1.c |  8 ++++++++
>  tools/testing/selftests/bpf/progs/linked_funcs2.c |  8 ++++++++
>  4 files changed, 30 insertions(+), 7 deletions(-)
>
> --
> 2.30.2
>

Re: [PATCH bpf-next 0/3] Fix handling of CO-RE relos for __weak subprogs

[patchwork-bot+netdevbpf]

Hello:

This series was applied to bpf/bpf-next.git (master)
by Daniel Borkmann <daniel@iogearbox.net>:

On Fri, 8 Apr 2022 11:14:22 -0700 you wrote:
> Fix the issue accidentally discovered during libbpf USDT support testing.
> Libbpf overzealously complained about CO-RE relocations belonging to the code
> of a __weak subprog that got overriden by another instance of that function.
> 
> Fix the issue fixed, return back to __weak __hidden annotation for USDT
> BPF-side APIs.
> 
> [...]

Here is the summary with links:
  - [bpf-next,1/3] libbpf: don't error out on CO-RE relos for overriden weak subprogs
    https://git.kernel.org/bpf/bpf-next/c/e89d57d938c8
  - [bpf-next,2/3] libbpf: use weak hidden modifier for USDT BPF-side API functions
    https://git.kernel.org/bpf/bpf-next/c/2fa5b0f290e1
  - [bpf-next,3/3] selftests/bpf: add CO-RE relos into linked_funcs selftests
    https://git.kernel.org/bpf/bpf-next/c/8555defe4861

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