[PATCH bpf-next 00/10] Teach libbpf to "fix up" BPF verifier log

[PATCH bpf-next 00/10] Teach libbpf to "fix up" BPF verifier log

[Andrii Nakryiko]

This patch set teaches libbpf to enhance BPF verifier log with human-readable
and relevant information about failed CO-RE relocation. Patch #9 is the main
one with the new logic. See relevant commit messages for some more details.

All the other patches are either fixing various bugs detected
while working on this feature, most prominently a bug with libbpf not handling
CO-RE relocations for SEC("?...") programs, or are refactoring libbpf
internals to allow for easier reuse of CO-RE relo lookup and formatting logic.

Andrii Nakryiko (10):
  libbpf: fix anonymous type check in CO-RE logic
  libbpf: drop unhelpful "program too large" guess
  libbpf: fix logic for finding matching program for CO-RE relocation
  libbpf: avoid joining .BTF.ext data with BPF programs by section name
  selftests/bpf: add CO-RE relos and SEC("?...") to linked_funcs
    selftests
  libbpf: record subprog-resolved CO-RE relocations unconditionally
  libbpf: refactor CO-RE relo human description formatting routine
  libbpf: simplify bpf_core_parse_spec() signature
  libbpf: fix up verifier log for unguarded failed CO-RE relos
  selftests/bpf: add libbpf's log fixup logic selftests

 tools/lib/bpf/btf.c                           |   9 +-
 tools/lib/bpf/libbpf.c                        | 252 ++++++++++++++----
 tools/lib/bpf/libbpf_internal.h               |   7 +
 tools/lib/bpf/relo_core.c                     | 104 ++++----
 tools/lib/bpf/relo_core.h                     |   6 +
 .../selftests/bpf/prog_tests/linked_funcs.c   |   6 +
 .../selftests/bpf/prog_tests/log_fixup.c      | 114 ++++++++
 .../selftests/bpf/progs/linked_funcs1.c       |   7 +-
 .../selftests/bpf/progs/linked_funcs2.c       |   7 +-
 .../selftests/bpf/progs/test_log_fixup.c      |  38 +++
 tools/testing/selftests/bpf/test_progs.h      |  11 +
 11 files changed, 464 insertions(+), 97 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/log_fixup.c
 create mode 100644 tools/testing/selftests/bpf/progs/test_log_fixup.c

-- 
2.30.2

[PATCH bpf-next 01/10] libbpf: fix anonymous type check in CO-RE logic

[Andrii Nakryiko]

Use type name for checking whether CO-RE relocation is referring to
anonymous type. Using spec string makes no sense.

Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
 tools/lib/bpf/relo_core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/lib/bpf/relo_core.c b/tools/lib/bpf/relo_core.c
index f946f23eab20..adaa22160692 100644
--- a/tools/lib/bpf/relo_core.c
+++ b/tools/lib/bpf/relo_core.c
@@ -1207,7 +1207,7 @@ int bpf_core_calc_relo_insn(const char *prog_name,
 	}
 
 	/* libbpf doesn't support candidate search for anonymous types */
-	if (str_is_empty(spec_str)) {
+	if (str_is_empty(local_name)) {
 		pr_warn("prog '%s': relo #%d: <%s> (%d) relocation doesn't support anonymous types\n",
 			prog_name, relo_idx, core_relo_kind_str(relo->kind), relo->kind);
 		return -EOPNOTSUPP;
-- 
2.30.2

[PATCH bpf-next 02/10] libbpf: drop unhelpful "program too large" guess

[Andrii Nakryiko]

libbpf pretends it knows actual limit of BPF program instructions based
on UAPI headers it compiled with. There is neither any guarantee that
UAPI headers match host kernel, nor BPF verifier actually uses
BPF_MAXINSNS constant anymore. Just drop unhelpful "guess", BPF verifier
will emit actual reason for failure in its logs anyways.

Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
 tools/lib/bpf/libbpf.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 873a29ce7781..5f2750af0e27 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -6828,10 +6828,6 @@ static int bpf_object_load_prog_instance(struct bpf_object *obj, struct bpf_prog
 		pr_warn("prog '%s': -- BEGIN PROG LOAD LOG --\n%s-- END PROG LOAD LOG --\n",
 			prog->name, log_buf);
 	}
-	if (insns_cnt >= BPF_MAXINSNS) {
-		pr_warn("prog '%s': program too large (%d insns), at most %d insns\n",
-			prog->name, insns_cnt, BPF_MAXINSNS);
-	}
 
 out:
 	if (own_log_buf)
-- 
2.30.2

[PATCH bpf-next 03/10] libbpf: fix logic for finding matching program for CO-RE relocation

[Andrii Nakryiko]

Fix the bug in bpf_object__relocate_core() which can lead to finding
invalid matching BPF program when processing CO-RE relocation. IF
matching program is not found, last encountered program will be assumed
to be correct program and thus error detection won't detect the problem.

Fixes: 9c82a63cf370 ("libbpf: Fix CO-RE relocs against .text section")
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
 tools/lib/bpf/libbpf.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 5f2750af0e27..840a26428937 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -5677,9 +5677,10 @@ bpf_object__relocate_core(struct bpf_object *obj, const char *targ_btf_path)
 		 */
 		prog = NULL;
 		for (i = 0; i < obj->nr_programs; i++) {
-			prog = &obj->programs[i];
-			if (strcmp(prog->sec_name, sec_name) == 0)
+			if (strcmp(obj->programs[i].sec_name, sec_name) == 0) {
+				prog = &obj->programs[i];
 				break;
+			}
 		}
 		if (!prog) {
 			pr_warn("sec '%s': failed to find a BPF program\n", sec_name);
-- 
2.30.2

[PATCH bpf-next 04/10] libbpf: avoid joining .BTF.ext data with BPF programs by section name

[Andrii Nakryiko]

Instead of using ELF section names as a joining key between .BTF.ext and
corresponding BPF programs, pre-build .BTF.ext section number to ELF
section index mapping during bpf_object__open() and use it later for
matching .BTF.ext information (func/line info or CO-RE relocations) to
their respective BPF programs and subprograms.

This simplifies corresponding joining logic and let's libbpf do
manipulations with BPF program's ELF sections like dropping leading '?'
character for non-autoloaded programs. Original joining logic in
bpf_object__relocate_core() (see relevant comment that's now removed)
was never elegant, so it's a good improvement regardless. But it also
avoids unnecessary internal assumptions about preserving original ELF
section name as BPF program's section name (which was broken when
SEC("?abc") support was added).

Fixes: a3820c481112 ("libbpf: Support opting out from autoloading BPF programs declaratively")
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
 tools/lib/bpf/btf.c             |  9 +++-
 tools/lib/bpf/libbpf.c          | 78 +++++++++++++++++++++------------
 tools/lib/bpf/libbpf_internal.h |  7 +++
 3 files changed, 65 insertions(+), 29 deletions(-)

diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c
index d124e9e533f0..bb1e06eb1eca 100644
--- a/tools/lib/bpf/btf.c
+++ b/tools/lib/bpf/btf.c
@@ -2626,6 +2626,7 @@ static int btf_ext_setup_info(struct btf_ext *btf_ext,
 	const struct btf_ext_info_sec *sinfo;
 	struct btf_ext_info *ext_info;
 	__u32 info_left, record_size;
+	size_t sec_cnt = 0;
 	/* The start of the info sec (including the __u32 record_size). */
 	void *info;
 
@@ -2689,8 +2690,7 @@ static int btf_ext_setup_info(struct btf_ext *btf_ext,
 			return -EINVAL;
 		}
 
-		total_record_size = sec_hdrlen +
-				    (__u64)num_records * record_size;
+		total_record_size = sec_hdrlen + (__u64)num_records * record_size;
 		if (info_left < total_record_size) {
 			pr_debug("%s section has incorrect num_records in .BTF.ext\n",
 			     ext_sec->desc);
@@ -2699,12 +2699,14 @@ static int btf_ext_setup_info(struct btf_ext *btf_ext,
 
 		info_left -= total_record_size;
 		sinfo = (void *)sinfo + total_record_size;
+		sec_cnt++;
 	}
 
 	ext_info = ext_sec->ext_info;
 	ext_info->len = ext_sec->len - sizeof(__u32);
 	ext_info->rec_size = record_size;
 	ext_info->info = info + sizeof(__u32);
+	ext_info->sec_cnt = sec_cnt;
 
 	return 0;
 }
@@ -2788,6 +2790,9 @@ void btf_ext__free(struct btf_ext *btf_ext)
 {
 	if (IS_ERR_OR_NULL(btf_ext))
 		return;
+	free(btf_ext->func_info.sec_idxs);
+	free(btf_ext->line_info.sec_idxs);
+	free(btf_ext->core_relo_info.sec_idxs);
 	free(btf_ext->data);
 	free(btf_ext);
 }
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 840a26428937..efc1ea91e12e 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -2765,6 +2765,9 @@ static int bpf_object__init_btf(struct bpf_object *obj,
 		btf__set_pointer_size(obj->btf, 8);
 	}
 	if (btf_ext_data) {
+		struct btf_ext_info *ext_segs[3];
+		int seg_num, sec_num;
+
 		if (!obj->btf) {
 			pr_debug("Ignore ELF section %s because its depending ELF section %s is not found.\n",
 				 BTF_EXT_ELF_SEC, BTF_ELF_SEC);
@@ -2778,6 +2781,43 @@ static int bpf_object__init_btf(struct bpf_object *obj,
 			obj->btf_ext = NULL;
 			goto out;
 		}
+
+		/* setup .BTF.ext to ELF section mapping */
+		ext_segs[0] = &obj->btf_ext->func_info;
+		ext_segs[1] = &obj->btf_ext->line_info;
+		ext_segs[2] = &obj->btf_ext->core_relo_info;
+		for (seg_num = 0; seg_num < ARRAY_SIZE(ext_segs); seg_num++) {
+			struct btf_ext_info *seg = ext_segs[seg_num];
+			const struct btf_ext_info_sec *sec;
+			const char *sec_name;
+			Elf_Scn *scn;
+
+			if (seg->sec_cnt == 0)
+				continue;
+
+			seg->sec_idxs = calloc(seg->sec_cnt, sizeof(*seg->sec_idxs));
+			if (!seg->sec_idxs) {
+				err = -ENOMEM;
+				goto out;
+			}
+
+			sec_num = 0;
+			for_each_btf_ext_sec(seg, sec) {
+				/* preventively increment index to avoid doing
+				 * this before every continue below
+				 */
+				sec_num++;
+
+				sec_name = btf__name_by_offset(obj->btf, sec->sec_name_off);
+				if (str_is_empty(sec_name))
+					continue;
+				scn = elf_sec_by_name(obj, sec_name);
+				if (!scn)
+					continue;
+
+				seg->sec_idxs[sec_num - 1] = elf_ndxscn(scn);
+			}
+		}
 	}
 out:
 	if (err && libbpf_needs_btf(obj)) {
@@ -5642,7 +5682,7 @@ bpf_object__relocate_core(struct bpf_object *obj, const char *targ_btf_path)
 	struct bpf_program *prog;
 	struct bpf_insn *insn;
 	const char *sec_name;
-	int i, err = 0, insn_idx, sec_idx;
+	int i, err = 0, insn_idx, sec_idx, sec_num;
 
 	if (obj->btf_ext->core_relo_info.len == 0)
 		return 0;
@@ -5663,33 +5703,18 @@ bpf_object__relocate_core(struct bpf_object *obj, const char *targ_btf_path)
 	}
 
 	seg = &obj->btf_ext->core_relo_info;
+	sec_num = 0;
 	for_each_btf_ext_sec(seg, sec) {
+		sec_idx = seg->sec_idxs[sec_num];
+		sec_num++;
+
 		sec_name = btf__name_by_offset(obj->btf, sec->sec_name_off);
 		if (str_is_empty(sec_name)) {
 			err = -EINVAL;
 			goto out;
 		}
-		/* bpf_object's ELF is gone by now so it's not easy to find
-		 * section index by section name, but we can find *any*
-		 * bpf_program within desired section name and use it's
-		 * prog->sec_idx to do a proper search by section index and
-		 * instruction offset
-		 */
-		prog = NULL;
-		for (i = 0; i < obj->nr_programs; i++) {
-			if (strcmp(obj->programs[i].sec_name, sec_name) == 0) {
-				prog = &obj->programs[i];
-				break;
-			}
-		}
-		if (!prog) {
-			pr_warn("sec '%s': failed to find a BPF program\n", sec_name);
-			return -ENOENT;
-		}
-		sec_idx = prog->sec_idx;
 
-		pr_debug("sec '%s': found %d CO-RE relocations\n",
-			 sec_name, sec->num_info);
+		pr_debug("sec '%s': found %d CO-RE relocations\n", sec_name, sec->num_info);
 
 		for_each_btf_ext_rec(seg, sec, i, rec) {
 			if (rec->insn_off % BPF_INSN_SZ)
@@ -5873,14 +5898,13 @@ static int adjust_prog_btf_ext_info(const struct bpf_object *obj,
 	void *rec, *rec_end, *new_prog_info;
 	const struct btf_ext_info_sec *sec;
 	size_t old_sz, new_sz;
-	const char *sec_name;
-	int i, off_adj;
+	int i, sec_num, sec_idx, off_adj;
 
+	sec_num = 0;
 	for_each_btf_ext_sec(ext_info, sec) {
-		sec_name = btf__name_by_offset(obj->btf, sec->sec_name_off);
-		if (!sec_name)
-			return -EINVAL;
-		if (strcmp(sec_name, prog->sec_name) != 0)
+		sec_idx = ext_info->sec_idxs[sec_num];
+		sec_num++;
+		if (prog->sec_idx != sec_idx)
 			continue;
 
 		for_each_btf_ext_rec(ext_info, sec, i, rec) {
diff --git a/tools/lib/bpf/libbpf_internal.h b/tools/lib/bpf/libbpf_internal.h
index 054cd8e93d7c..4abdbe2fea9d 100644
--- a/tools/lib/bpf/libbpf_internal.h
+++ b/tools/lib/bpf/libbpf_internal.h
@@ -376,6 +376,13 @@ struct btf_ext_info {
 	void *info;
 	__u32 rec_size;
 	__u32 len;
+	/* optional (maintained internally by libbpf) mapping between .BTF.ext
+	 * section and corresponding ELF section. This is used to join
+	 * information like CO-RE relocation records with corresponding BPF
+	 * programs defined in ELF sections
+	 */
+	__u32 *sec_idxs;
+	int sec_cnt;
 };
 
 #define for_each_btf_ext_sec(seg, sec)					\
-- 
2.30.2

[PATCH bpf-next 05/10] selftests/bpf: add CO-RE relos and SEC("?...") to linked_funcs selftests

[Andrii Nakryiko]

Enhance linked_funcs selftest with two tricky features that might not
obviously work correctly together. We add CO-RE relocations to entry BPF
programs and mark those programs as non-autoloadable with SEC("?...")
annotation. This makes sure that libbpf itself handles .BTF.ext CO-RE
relocation data matching correctly for SEC("?...") programs, as well as
ensures that BPF static linker handles this correctly (this was the case
before, no changes are necessary, but it wasn't explicitly tested).

Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
 tools/testing/selftests/bpf/prog_tests/linked_funcs.c | 6 ++++++
 tools/testing/selftests/bpf/progs/linked_funcs1.c     | 7 ++++++-
 tools/testing/selftests/bpf/progs/linked_funcs2.c     | 7 ++++++-
 3 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/linked_funcs.c b/tools/testing/selftests/bpf/prog_tests/linked_funcs.c
index e9916f2817ec..cad664546912 100644
--- a/tools/testing/selftests/bpf/prog_tests/linked_funcs.c
+++ b/tools/testing/selftests/bpf/prog_tests/linked_funcs.c
@@ -14,6 +14,12 @@ void test_linked_funcs(void)
 	if (!ASSERT_OK_PTR(skel, "skel_open"))
 		return;
 
+	/* handler1 and handler2 are marked as SEC("?raw_tp/sys_enter") and
+	 * are set to not autoload by default
+	 */
+	bpf_program__set_autoload(skel->progs.handler1, true);
+	bpf_program__set_autoload(skel->progs.handler2, true);
+
 	skel->rodata->my_tid = syscall(SYS_gettid);
 	skel->bss->syscall_id = SYS_getpgid;
 
diff --git a/tools/testing/selftests/bpf/progs/linked_funcs1.c b/tools/testing/selftests/bpf/progs/linked_funcs1.c
index 963b393c37e8..b05571bc67d5 100644
--- a/tools/testing/selftests/bpf/progs/linked_funcs1.c
+++ b/tools/testing/selftests/bpf/progs/linked_funcs1.c
@@ -61,12 +61,17 @@ extern int set_output_val2(int x);
 /* here we'll force set_output_ctx2() to be __hidden in the final obj file */
 __hidden extern void set_output_ctx2(__u64 *ctx);
 
-SEC("raw_tp/sys_enter")
+SEC("?raw_tp/sys_enter")
 int BPF_PROG(handler1, struct pt_regs *regs, long id)
 {
+	static volatile int whatever;
+
 	if (my_tid != (u32)bpf_get_current_pid_tgid() || id != syscall_id)
 		return 0;
 
+	/* make sure we have CO-RE relocations in main program */
+	whatever = bpf_core_type_size(struct task_struct);
+
 	set_output_val2(1000);
 	set_output_ctx2(ctx); /* ctx definition is hidden in BPF_PROG macro */
 
diff --git a/tools/testing/selftests/bpf/progs/linked_funcs2.c b/tools/testing/selftests/bpf/progs/linked_funcs2.c
index db195872f4eb..ee7e3848ee4f 100644
--- a/tools/testing/selftests/bpf/progs/linked_funcs2.c
+++ b/tools/testing/selftests/bpf/progs/linked_funcs2.c
@@ -61,12 +61,17 @@ extern int set_output_val1(int x);
 /* here we'll force set_output_ctx1() to be __hidden in the final obj file */
 __hidden extern void set_output_ctx1(__u64 *ctx);
 
-SEC("raw_tp/sys_enter")
+SEC("?raw_tp/sys_enter")
 int BPF_PROG(handler2, struct pt_regs *regs, long id)
 {
+	static volatile int whatever;
+
 	if (my_tid != (u32)bpf_get_current_pid_tgid() || id != syscall_id)
 		return 0;
 
+	/* make sure we have CO-RE relocations in main program */
+	whatever = bpf_core_type_size(struct task_struct);
+
 	set_output_val1(2000);
 	set_output_ctx1(ctx); /* ctx definition is hidden in BPF_PROG macro */
 
-- 
2.30.2

[PATCH bpf-next 06/10] libbpf: record subprog-resolved CO-RE relocations unconditionally

[Andrii Nakryiko]

Previously, libbpf recorded CO-RE relocations with insns_idx resolved
according to finalized subprog locations (which are appended at the end
of entry BPF program) to simplify the job of light skeleton generator.

This is necessary because once subprogs' instructions are appended to
main entry BPF program all the subprog instruction indices are shifted
and that shift is different for each entry (main) BPF program, so it's
generally impossible to map final absolute insn_idx of the finalized BPF
program to their original locations inside subprograms.

This information is now going to be used not only during light skeleton
generation, but also to map absolute instruction index to subprog's
instruction and its corresponding CO-RE relocation. So start recording
these relocations always, not just when obj->gen_loader is set.

This information is going to be freed at the end of bpf_object__load()
step, as before (but this can change in the future if there will be
a need for this information post load step).

Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
 tools/lib/bpf/libbpf.c | 27 ++++++++++++---------------
 1 file changed, 12 insertions(+), 15 deletions(-)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index efc1ea91e12e..5ebbfe8b5e1c 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -5749,16 +5749,16 @@ bpf_object__relocate_core(struct bpf_object *obj, const char *targ_btf_path)
 				return -EINVAL;
 			insn = &prog->insns[insn_idx];
 
-			if (prog->obj->gen_loader) {
-				err = record_relo_core(prog, rec, insn_idx);
-				if (err) {
-					pr_warn("prog '%s': relo #%d: failed to record relocation: %d\n",
-						prog->name, i, err);
-					goto out;
-				}
-				continue;
+			err = record_relo_core(prog, rec, insn_idx);
+			if (err) {
+				pr_warn("prog '%s': relo #%d: failed to record relocation: %d\n",
+					prog->name, i, err);
+				goto out;
 			}
 
+			if (prog->obj->gen_loader)
+				continue;
+
 			err = bpf_core_resolve_relo(prog, rec, i, obj->btf, cand_cache, &targ_res);
 			if (err) {
 				pr_warn("prog '%s': relo #%d: failed to relocate: %d\n",
@@ -6299,7 +6299,6 @@ bpf_object__relocate_calls(struct bpf_object *obj, struct bpf_program *prog)
 	if (err)
 		return err;
 
-
 	return 0;
 }
 
@@ -6360,8 +6359,7 @@ bpf_object__relocate(struct bpf_object *obj, const char *targ_btf_path)
 				err);
 			return err;
 		}
-		if (obj->gen_loader)
-			bpf_object__sort_relos(obj);
+		bpf_object__sort_relos(obj);
 	}
 
 	/* Before relocating calls pre-process relocations and mark
@@ -6421,8 +6419,7 @@ bpf_object__relocate(struct bpf_object *obj, const char *targ_btf_path)
 			return err;
 		}
 	}
-	if (!obj->gen_loader)
-		bpf_object__free_relocs(obj);
+
 	return 0;
 }
 
@@ -7014,8 +7011,8 @@ bpf_object__load_progs(struct bpf_object *obj, int log_level)
 		if (err)
 			return err;
 	}
-	if (obj->gen_loader)
-		bpf_object__free_relocs(obj);
+
+	bpf_object__free_relocs(obj);
 	return 0;
 }
 
-- 
2.30.2

[PATCH bpf-next 07/10] libbpf: refactor CO-RE relo human description formatting routine

[Andrii Nakryiko]

Refactor how CO-RE relocation is formatted. Now it dumps human-readable
representation, currently used by libbpf in either debug or error
message output during CO-RE relocation resolution process, into provided
buffer. This approach allows for better reuse of this functionality
outside of CO-RE relocation resolution, which we'll use in next patch
for providing better error message for BPF verifier rejecting BPF
program due to unguarded failed CO-RE relocation.

It also gets rid of annoying "stitching" of libbpf_print() calls, which
was the only place where we did this.

Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
 tools/lib/bpf/relo_core.c | 64 +++++++++++++++++++++++----------------
 1 file changed, 38 insertions(+), 26 deletions(-)

diff --git a/tools/lib/bpf/relo_core.c b/tools/lib/bpf/relo_core.c
index adaa22160692..13d36a705464 100644
--- a/tools/lib/bpf/relo_core.c
+++ b/tools/lib/bpf/relo_core.c
@@ -1055,51 +1055,66 @@ int bpf_core_patch_insn(const char *prog_name, struct bpf_insn *insn,
  * [<type-id>] (<type-name>) + <raw-spec> => <offset>@<spec>,
  * where <spec> is a C-syntax view of recorded field access, e.g.: x.a[3].b
  */
-static void bpf_core_dump_spec(const char *prog_name, int level, const struct bpf_core_spec *spec)
+static int bpf_core_format_spec(char *buf, size_t buf_sz, const struct bpf_core_spec *spec)
 {
 	const struct btf_type *t;
 	const struct btf_enum *e;
 	const char *s;
 	__u32 type_id;
-	int i;
+	int i, len = 0;
+
+#define append_buf(fmt, args...)				\
+	({							\
+		int r;						\
+		r = snprintf(buf, buf_sz, fmt, ##args);		\
+		len += r;					\
+		if (r >= buf_sz)				\
+			r = buf_sz;				\
+		buf += r;					\
+		buf_sz -= r;					\
+	})
 
 	type_id = spec->root_type_id;
 	t = btf_type_by_id(spec->btf, type_id);
 	s = btf__name_by_offset(spec->btf, t->name_off);
 
-	libbpf_print(level, "[%u] %s %s", type_id, btf_kind_str(t), str_is_empty(s) ? "<anon>" : s);
+	append_buf("<%s> [%u] %s %s",
+		   core_relo_kind_str(spec->relo_kind),
+		   type_id, btf_kind_str(t), str_is_empty(s) ? "<anon>" : s);
 
 	if (core_relo_is_type_based(spec->relo_kind))
-		return;
+		return len;
 
 	if (core_relo_is_enumval_based(spec->relo_kind)) {
 		t = skip_mods_and_typedefs(spec->btf, type_id, NULL);
 		e = btf_enum(t) + spec->raw_spec[0];
 		s = btf__name_by_offset(spec->btf, e->name_off);
 
-		libbpf_print(level, "::%s = %u", s, e->val);
-		return;
+		append_buf("::%s = %u", s, e->val);
+		return len;
 	}
 
 	if (core_relo_is_field_based(spec->relo_kind)) {
 		for (i = 0; i < spec->len; i++) {
 			if (spec->spec[i].name)
-				libbpf_print(level, ".%s", spec->spec[i].name);
+				append_buf(".%s", spec->spec[i].name);
 			else if (i > 0 || spec->spec[i].idx > 0)
-				libbpf_print(level, "[%u]", spec->spec[i].idx);
+				append_buf("[%u]", spec->spec[i].idx);
 		}
 
-		libbpf_print(level, " (");
+		append_buf(" (");
 		for (i = 0; i < spec->raw_len; i++)
-			libbpf_print(level, "%s%d", i == 0 ? "" : ":", spec->raw_spec[i]);
+			append_buf("%s%d", i == 0 ? "" : ":", spec->raw_spec[i]);
 
 		if (spec->bit_offset % 8)
-			libbpf_print(level, " @ offset %u.%u)",
-				     spec->bit_offset / 8, spec->bit_offset % 8);
+			append_buf(" @ offset %u.%u)", spec->bit_offset / 8, spec->bit_offset % 8);
 		else
-			libbpf_print(level, " @ offset %u)", spec->bit_offset / 8);
-		return;
+			append_buf(" @ offset %u)", spec->bit_offset / 8);
+		return len;
 	}
+
+	return len;
+#undef append_buf
 }
 
 /*
@@ -1168,6 +1183,7 @@ int bpf_core_calc_relo_insn(const char *prog_name,
 	const char *local_name;
 	__u32 local_id;
 	const char *spec_str;
+	char spec_buf[256];
 	int i, j, err;
 
 	local_id = relo->type_id;
@@ -1190,10 +1206,8 @@ int bpf_core_calc_relo_insn(const char *prog_name,
 		return -EINVAL;
 	}
 
-	pr_debug("prog '%s': relo #%d: kind <%s> (%d), spec is ", prog_name,
-		 relo_idx, core_relo_kind_str(relo->kind), relo->kind);
-	bpf_core_dump_spec(prog_name, LIBBPF_DEBUG, local_spec);
-	libbpf_print(LIBBPF_DEBUG, "\n");
+	bpf_core_format_spec(spec_buf, sizeof(spec_buf), local_spec);
+	pr_debug("prog '%s': relo #%d: %s\n", prog_name, relo_idx, spec_buf);
 
 	/* TYPE_ID_LOCAL relo is special and doesn't need candidate search */
 	if (relo->kind == BPF_CORE_TYPE_ID_LOCAL) {
@@ -1217,17 +1231,15 @@ int bpf_core_calc_relo_insn(const char *prog_name,
 		err = bpf_core_spec_match(local_spec, cands->cands[i].btf,
 					  cands->cands[i].id, cand_spec);
 		if (err < 0) {
-			pr_warn("prog '%s': relo #%d: error matching candidate #%d ",
-				prog_name, relo_idx, i);
-			bpf_core_dump_spec(prog_name, LIBBPF_WARN, cand_spec);
-			libbpf_print(LIBBPF_WARN, ": %d\n", err);
+			bpf_core_format_spec(spec_buf, sizeof(spec_buf), cand_spec);
+			pr_warn("prog '%s': relo #%d: error matching candidate #%d %s: %d\n ",
+				prog_name, relo_idx, i, spec_buf, err);
 			return err;
 		}
 
-		pr_debug("prog '%s': relo #%d: %s candidate #%d ", prog_name,
-			 relo_idx, err == 0 ? "non-matching" : "matching", i);
-		bpf_core_dump_spec(prog_name, LIBBPF_DEBUG, cand_spec);
-		libbpf_print(LIBBPF_DEBUG, "\n");
+		bpf_core_format_spec(spec_buf, sizeof(spec_buf), cand_spec);
+		pr_debug("prog '%s': relo #%d: %s candidate #%d %s\n", prog_name,
+			 relo_idx, err == 0 ? "non-matching" : "matching", i, spec_buf);
 
 		if (err == 0)
 			continue;
-- 
2.30.2

Re: [PATCH bpf-next 07/10] libbpf: refactor CO-RE relo human description formatting routine

[Alexei Starovoitov]

On Mon, Apr 25, 2022 at 05:45:08PM -0700, Andrii Nakryiko wrote:
> Refactor how CO-RE relocation is formatted. Now it dumps human-readable
> representation, currently used by libbpf in either debug or error
> message output during CO-RE relocation resolution process, into provided
> buffer. This approach allows for better reuse of this functionality
> outside of CO-RE relocation resolution, which we'll use in next patch
> for providing better error message for BPF verifier rejecting BPF
> program due to unguarded failed CO-RE relocation.
> 
> It also gets rid of annoying "stitching" of libbpf_print() calls, which
> was the only place where we did this.
> 
> Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
> ---
>  tools/lib/bpf/relo_core.c | 64 +++++++++++++++++++++++----------------
>  1 file changed, 38 insertions(+), 26 deletions(-)
> 
> diff --git a/tools/lib/bpf/relo_core.c b/tools/lib/bpf/relo_core.c
> index adaa22160692..13d36a705464 100644
> --- a/tools/lib/bpf/relo_core.c
> +++ b/tools/lib/bpf/relo_core.c
> @@ -1055,51 +1055,66 @@ int bpf_core_patch_insn(const char *prog_name, struct bpf_insn *insn,
>   * [<type-id>] (<type-name>) + <raw-spec> => <offset>@<spec>,
>   * where <spec> is a C-syntax view of recorded field access, e.g.: x.a[3].b
>   */
> -static void bpf_core_dump_spec(const char *prog_name, int level, const struct bpf_core_spec *spec)
> +static int bpf_core_format_spec(char *buf, size_t buf_sz, const struct bpf_core_spec *spec)
>  {
>  	const struct btf_type *t;
>  	const struct btf_enum *e;
>  	const char *s;
>  	__u32 type_id;
> -	int i;
> +	int i, len = 0;
> +
> +#define append_buf(fmt, args...)				\
> +	({							\
> +		int r;						\
> +		r = snprintf(buf, buf_sz, fmt, ##args);		\
> +		len += r;					\
> +		if (r >= buf_sz)				\

Do we need to check for r<0 here too or it's highly unlikely?

> +			r = buf_sz;				\
> +		buf += r;					\
> +		buf_sz -= r;					\
> +	})
>  
>  	type_id = spec->root_type_id;
>  	t = btf_type_by_id(spec->btf, type_id);
>  	s = btf__name_by_offset(spec->btf, t->name_off);
>  
> -	libbpf_print(level, "[%u] %s %s", type_id, btf_kind_str(t), str_is_empty(s) ? "<anon>" : s);
> +	append_buf("<%s> [%u] %s %s",
> +		   core_relo_kind_str(spec->relo_kind),
> +		   type_id, btf_kind_str(t), str_is_empty(s) ? "<anon>" : s);
>  
>  	if (core_relo_is_type_based(spec->relo_kind))
> -		return;
> +		return len;
>  
>  	if (core_relo_is_enumval_based(spec->relo_kind)) {
>  		t = skip_mods_and_typedefs(spec->btf, type_id, NULL);
>  		e = btf_enum(t) + spec->raw_spec[0];
>  		s = btf__name_by_offset(spec->btf, e->name_off);
>  
> -		libbpf_print(level, "::%s = %u", s, e->val);
> -		return;
> +		append_buf("::%s = %u", s, e->val);
> +		return len;
>  	}
>  
>  	if (core_relo_is_field_based(spec->relo_kind)) {
>  		for (i = 0; i < spec->len; i++) {
>  			if (spec->spec[i].name)
> -				libbpf_print(level, ".%s", spec->spec[i].name);
> +				append_buf(".%s", spec->spec[i].name);
>  			else if (i > 0 || spec->spec[i].idx > 0)
> -				libbpf_print(level, "[%u]", spec->spec[i].idx);
> +				append_buf("[%u]", spec->spec[i].idx);
>  		}
>  
> -		libbpf_print(level, " (");
> +		append_buf(" (");
>  		for (i = 0; i < spec->raw_len; i++)
> -			libbpf_print(level, "%s%d", i == 0 ? "" : ":", spec->raw_spec[i]);
> +			append_buf("%s%d", i == 0 ? "" : ":", spec->raw_spec[i]);
>  
>  		if (spec->bit_offset % 8)
> -			libbpf_print(level, " @ offset %u.%u)",
> -				     spec->bit_offset / 8, spec->bit_offset % 8);
> +			append_buf(" @ offset %u.%u)", spec->bit_offset / 8, spec->bit_offset % 8);
>  		else
> -			libbpf_print(level, " @ offset %u)", spec->bit_offset / 8);
> -		return;
> +			append_buf(" @ offset %u)", spec->bit_offset / 8);
> +		return len;
>  	}
> +
> +	return len;
> +#undef append_buf
>  }
>  
>  /*
> @@ -1168,6 +1183,7 @@ int bpf_core_calc_relo_insn(const char *prog_name,
>  	const char *local_name;
>  	__u32 local_id;
>  	const char *spec_str;
> +	char spec_buf[256];
>  	int i, j, err;
>  
>  	local_id = relo->type_id;
> @@ -1190,10 +1206,8 @@ int bpf_core_calc_relo_insn(const char *prog_name,
>  		return -EINVAL;
>  	}
>  
> -	pr_debug("prog '%s': relo #%d: kind <%s> (%d), spec is ", prog_name,
> -		 relo_idx, core_relo_kind_str(relo->kind), relo->kind);
> -	bpf_core_dump_spec(prog_name, LIBBPF_DEBUG, local_spec);
> -	libbpf_print(LIBBPF_DEBUG, "\n");
> +	bpf_core_format_spec(spec_buf, sizeof(spec_buf), local_spec);
> +	pr_debug("prog '%s': relo #%d: %s\n", prog_name, relo_idx, spec_buf);

Looks great, but return value 'len' doesn't seem to be used in this
patch or in the following patch.
What was the intent ?

Re: [PATCH bpf-next 07/10] libbpf: refactor CO-RE relo human description formatting routine

[Andrii Nakryiko]

On Tue, Apr 26, 2022 at 11:52 AM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Mon, Apr 25, 2022 at 05:45:08PM -0700, Andrii Nakryiko wrote:
> > Refactor how CO-RE relocation is formatted. Now it dumps human-readable
> > representation, currently used by libbpf in either debug or error
> > message output during CO-RE relocation resolution process, into provided
> > buffer. This approach allows for better reuse of this functionality
> > outside of CO-RE relocation resolution, which we'll use in next patch
> > for providing better error message for BPF verifier rejecting BPF
> > program due to unguarded failed CO-RE relocation.
> >
> > It also gets rid of annoying "stitching" of libbpf_print() calls, which
> > was the only place where we did this.
> >
> > Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
> > ---
> >  tools/lib/bpf/relo_core.c | 64 +++++++++++++++++++++++----------------
> >  1 file changed, 38 insertions(+), 26 deletions(-)
> >
> > diff --git a/tools/lib/bpf/relo_core.c b/tools/lib/bpf/relo_core.c
> > index adaa22160692..13d36a705464 100644
> > --- a/tools/lib/bpf/relo_core.c
> > +++ b/tools/lib/bpf/relo_core.c
> > @@ -1055,51 +1055,66 @@ int bpf_core_patch_insn(const char *prog_name, struct bpf_insn *insn,
> >   * [<type-id>] (<type-name>) + <raw-spec> => <offset>@<spec>,
> >   * where <spec> is a C-syntax view of recorded field access, e.g.: x.a[3].b
> >   */
> > -static void bpf_core_dump_spec(const char *prog_name, int level, const struct bpf_core_spec *spec)
> > +static int bpf_core_format_spec(char *buf, size_t buf_sz, const struct bpf_core_spec *spec)
> >  {
> >       const struct btf_type *t;
> >       const struct btf_enum *e;
> >       const char *s;
> >       __u32 type_id;
> > -     int i;
> > +     int i, len = 0;
> > +
> > +#define append_buf(fmt, args...)                             \
> > +     ({                                                      \
> > +             int r;                                          \
> > +             r = snprintf(buf, buf_sz, fmt, ##args);         \
> > +             len += r;                                       \
> > +             if (r >= buf_sz)                                \
>
> Do we need to check for r<0 here too or it's highly unlikely?

I decided not to, as this would just mean an implementation bug
(invalid % formatter used in format string), which hopefully will be
caught in testing. It felt a bit too defensive, but if you think it's
better to check, then we can just have if (r < 0) return r; right
after snprintf()?

>
> > +                     r = buf_sz;                             \
> > +             buf += r;                                       \
> > +             buf_sz -= r;                                    \
> > +     })
> >
> >       type_id = spec->root_type_id;
> >       t = btf_type_by_id(spec->btf, type_id);
> >       s = btf__name_by_offset(spec->btf, t->name_off);
> >
> > -     libbpf_print(level, "[%u] %s %s", type_id, btf_kind_str(t), str_is_empty(s) ? "<anon>" : s);
> > +     append_buf("<%s> [%u] %s %s",
> > +                core_relo_kind_str(spec->relo_kind),
> > +                type_id, btf_kind_str(t), str_is_empty(s) ? "<anon>" : s);
> >
> >       if (core_relo_is_type_based(spec->relo_kind))
> > -             return;
> > +             return len;
> >
> >       if (core_relo_is_enumval_based(spec->relo_kind)) {
> >               t = skip_mods_and_typedefs(spec->btf, type_id, NULL);
> >               e = btf_enum(t) + spec->raw_spec[0];
> >               s = btf__name_by_offset(spec->btf, e->name_off);
> >
> > -             libbpf_print(level, "::%s = %u", s, e->val);
> > -             return;
> > +             append_buf("::%s = %u", s, e->val);
> > +             return len;
> >       }
> >
> >       if (core_relo_is_field_based(spec->relo_kind)) {
> >               for (i = 0; i < spec->len; i++) {
> >                       if (spec->spec[i].name)
> > -                             libbpf_print(level, ".%s", spec->spec[i].name);
> > +                             append_buf(".%s", spec->spec[i].name);
> >                       else if (i > 0 || spec->spec[i].idx > 0)
> > -                             libbpf_print(level, "[%u]", spec->spec[i].idx);
> > +                             append_buf("[%u]", spec->spec[i].idx);
> >               }
> >
> > -             libbpf_print(level, " (");
> > +             append_buf(" (");
> >               for (i = 0; i < spec->raw_len; i++)
> > -                     libbpf_print(level, "%s%d", i == 0 ? "" : ":", spec->raw_spec[i]);
> > +                     append_buf("%s%d", i == 0 ? "" : ":", spec->raw_spec[i]);
> >
> >               if (spec->bit_offset % 8)
> > -                     libbpf_print(level, " @ offset %u.%u)",
> > -                                  spec->bit_offset / 8, spec->bit_offset % 8);
> > +                     append_buf(" @ offset %u.%u)", spec->bit_offset / 8, spec->bit_offset % 8);
> >               else
> > -                     libbpf_print(level, " @ offset %u)", spec->bit_offset / 8);
> > -             return;
> > +                     append_buf(" @ offset %u)", spec->bit_offset / 8);
> > +             return len;
> >       }
> > +
> > +     return len;
> > +#undef append_buf
> >  }
> >
> >  /*
> > @@ -1168,6 +1183,7 @@ int bpf_core_calc_relo_insn(const char *prog_name,
> >       const char *local_name;
> >       __u32 local_id;
> >       const char *spec_str;
> > +     char spec_buf[256];
> >       int i, j, err;
> >
> >       local_id = relo->type_id;
> > @@ -1190,10 +1206,8 @@ int bpf_core_calc_relo_insn(const char *prog_name,
> >               return -EINVAL;
> >       }
> >
> > -     pr_debug("prog '%s': relo #%d: kind <%s> (%d), spec is ", prog_name,
> > -              relo_idx, core_relo_kind_str(relo->kind), relo->kind);
> > -     bpf_core_dump_spec(prog_name, LIBBPF_DEBUG, local_spec);
> > -     libbpf_print(LIBBPF_DEBUG, "\n");
> > +     bpf_core_format_spec(spec_buf, sizeof(spec_buf), local_spec);
> > +     pr_debug("prog '%s': relo #%d: %s\n", prog_name, relo_idx, spec_buf);
>
> Looks great, but return value 'len' doesn't seem to be used in this
> patch or in the following patch.
> What was the intent ?


Yeah, it's not used right now. I wanted to keep snprintf() semantics
where it returns what the length of output would be if we had big
enough buffer (I love that semantics in snprintf). But as it is right
now I just assume that the buffer is big enough and if not output will
be truncated (which is not a big deal in this case). It should be very
hard to fill up 256 characters with
some.struct.access.pattern.and.so.on :)

So it's a nice bonus behavior kept intentionally.

[PATCH bpf-next 08/10] libbpf: simplify bpf_core_parse_spec() signature

[Andrii Nakryiko]

Simplify bpf_core_parse_spec() signature to take struct bpf_core_relo as
an input instead of requiring callers to decompose them into type_id,
relo, spec_str, etc. This makes using and reusing this helper easier.

Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
 tools/lib/bpf/relo_core.c | 34 +++++++++++++++-------------------
 1 file changed, 15 insertions(+), 19 deletions(-)

diff --git a/tools/lib/bpf/relo_core.c b/tools/lib/bpf/relo_core.c
index 13d36a705464..4a9ad0cfb474 100644
--- a/tools/lib/bpf/relo_core.c
+++ b/tools/lib/bpf/relo_core.c
@@ -179,28 +179,27 @@ static bool core_relo_is_enumval_based(enum bpf_core_relo_kind kind)
  * string to specify enumerator's value index that need to be relocated.
  */
 static int bpf_core_parse_spec(const char *prog_name, const struct btf *btf,
-			       __u32 type_id,
-			       const char *spec_str,
-			       enum bpf_core_relo_kind relo_kind,
+			       const struct bpf_core_relo *relo,
 			       struct bpf_core_spec *spec)
 {
 	int access_idx, parsed_len, i;
 	struct bpf_core_accessor *acc;
 	const struct btf_type *t;
-	const char *name;
+	const char *name, *spec_str;
 	__u32 id;
 	__s64 sz;
 
+	spec_str = btf__name_by_offset(btf, relo->access_str_off);
 	if (str_is_empty(spec_str) || *spec_str == ':')
 		return -EINVAL;
 
 	memset(spec, 0, sizeof(*spec));
 	spec->btf = btf;
-	spec->root_type_id = type_id;
-	spec->relo_kind = relo_kind;
+	spec->root_type_id = relo->type_id;
+	spec->relo_kind = relo->kind;
 
 	/* type-based relocations don't have a field access string */
-	if (core_relo_is_type_based(relo_kind)) {
+	if (core_relo_is_type_based(relo->kind)) {
 		if (strcmp(spec_str, "0"))
 			return -EINVAL;
 		return 0;
@@ -221,7 +220,7 @@ static int bpf_core_parse_spec(const char *prog_name, const struct btf *btf,
 	if (spec->raw_len == 0)
 		return -EINVAL;
 
-	t = skip_mods_and_typedefs(btf, type_id, &id);
+	t = skip_mods_and_typedefs(btf, relo->type_id, &id);
 	if (!t)
 		return -EINVAL;
 
@@ -231,7 +230,7 @@ static int bpf_core_parse_spec(const char *prog_name, const struct btf *btf,
 	acc->idx = access_idx;
 	spec->len++;
 
-	if (core_relo_is_enumval_based(relo_kind)) {
+	if (core_relo_is_enumval_based(relo->kind)) {
 		if (!btf_is_enum(t) || spec->raw_len > 1 || access_idx >= btf_vlen(t))
 			return -EINVAL;
 
@@ -240,7 +239,7 @@ static int bpf_core_parse_spec(const char *prog_name, const struct btf *btf,
 		return 0;
 	}
 
-	if (!core_relo_is_field_based(relo_kind))
+	if (!core_relo_is_field_based(relo->kind))
 		return -EINVAL;
 
 	sz = btf__resolve_size(btf, id);
@@ -301,7 +300,7 @@ static int bpf_core_parse_spec(const char *prog_name, const struct btf *btf,
 			spec->bit_offset += access_idx * sz * 8;
 		} else {
 			pr_warn("prog '%s': relo for [%u] %s (at idx %d) captures type [%d] of unexpected kind %s\n",
-				prog_name, type_id, spec_str, i, id, btf_kind_str(t));
+				prog_name, relo->type_id, spec_str, i, id, btf_kind_str(t));
 			return -EINVAL;
 		}
 	}
@@ -1182,7 +1181,6 @@ int bpf_core_calc_relo_insn(const char *prog_name,
 	const struct btf_type *local_type;
 	const char *local_name;
 	__u32 local_id;
-	const char *spec_str;
 	char spec_buf[256];
 	int i, j, err;
 
@@ -1192,17 +1190,15 @@ int bpf_core_calc_relo_insn(const char *prog_name,
 	if (!local_name)
 		return -EINVAL;
 
-	spec_str = btf__name_by_offset(local_btf, relo->access_str_off);
-	if (str_is_empty(spec_str))
-		return -EINVAL;
-
-	err = bpf_core_parse_spec(prog_name, local_btf, local_id, spec_str,
-				  relo->kind, local_spec);
+	err = bpf_core_parse_spec(prog_name, local_btf, relo, local_spec);
 	if (err) {
+		const char *spec_str;
+
+		spec_str = btf__name_by_offset(local_btf, relo->access_str_off);
 		pr_warn("prog '%s': relo #%d: parsing [%d] %s %s + %s failed: %d\n",
 			prog_name, relo_idx, local_id, btf_kind_str(local_type),
 			str_is_empty(local_name) ? "<anon>" : local_name,
-			spec_str, err);
+			spec_str ?: "<?>", err);
 		return -EINVAL;
 	}
 
-- 
2.30.2

[PATCH bpf-next 09/10] libbpf: fix up verifier log for unguarded failed CO-RE relos

[Andrii Nakryiko]

Teach libbpf to post-process BPF verifier log on BPF program load
failure and detect known error patterns to provide user with more
context.

Currently there is one such common situation: an "unguarded" failed BPF
CO-RE relocation. While failing CO-RE relocation is expected, it is
expected to be property guarded in BPF code such that BPF verifier
always eliminates BPF instructions corresponding to such failed CO-RE
relos as dead code. In cases when user failed to take such precautions,
BPF verifier provides the best log it can:

  123: (85) call unknown#195896080
  invalid func unknown#195896080

Such incomprehensible log error is due to libbpf "poisoning" BPF
instruction that corresponds to failed CO-RE relocation by replacing it
with invalid `call 0xbad2310` instruction (195896080 == 0xbad2310 reads
"bad relo" if you squint hard enough).

Luckily, libbpf has all the necessary information to look up CO-RE
relocation that failed and provide more human-readable description of
what's going on:

  5: <invalid CO-RE relocation>
  failed to resolve CO-RE relocation <byte_off> [6] struct task_struct___bad.fake_field_subprog (0:2 @ offset 8)

This hopefully makes it much easier to understand what's wrong with
user's BPF program without googling magic constants.

This BPF verifier log fixup is setup to be extensible and is going to be
used for at least one other upcoming feature of libbpf in follow up patches.
Libbpf is parsing lines of BPF verifier log starting from the very end.
Currently it processes up to 10 lines of code looking for familiar
patterns. This avoids wasting lots of CPU processing huge verifier logs
(especially for log_level=2 verbosity level). Actual verification error
should normally be found in last few lines, so this should work
reliably.

If libbpf needs to expand log beyond available log_buf_size, it
truncates the end of the verifier log. Given verifier log normally ends
with something like:

  processed 2 insns (limit 1000000) max_states_per_insn 0 total_states 0 peak_states 0 mark_read 0

... truncating this on program load error isn't too bad (end user can
always increase log size, if it needs to get complete log).

Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
 tools/lib/bpf/libbpf.c    | 144 ++++++++++++++++++++++++++++++++++++++
 tools/lib/bpf/relo_core.c |   8 +--
 tools/lib/bpf/relo_core.h |   6 ++
 3 files changed, 154 insertions(+), 4 deletions(-)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 5ebbfe8b5e1c..7cb562f3c8bc 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -5626,6 +5626,22 @@ static int record_relo_core(struct bpf_program *prog,
 	return 0;
 }
 
+static const struct bpf_core_relo *find_relo_core(struct bpf_program *prog, int insn_idx)
+{
+	struct reloc_desc *relo;
+	int i;
+
+	for (i = 0; i < prog->nr_reloc; i++) {
+		relo = &prog->reloc_desc[i];
+		if (relo->type != RELO_CORE || relo->insn_idx != insn_idx)
+			continue;
+
+		return relo->core_relo;
+	}
+
+	return NULL;
+}
+
 static int bpf_core_resolve_relo(struct bpf_program *prog,
 				 const struct bpf_core_relo *relo,
 				 int relo_idx,
@@ -6696,6 +6712,8 @@ static int libbpf_prepare_prog_load(struct bpf_program *prog,
 	return 0;
 }
 
+static void fixup_verifier_log(struct bpf_program *prog, char *buf, size_t buf_sz);
+
 static int bpf_object_load_prog_instance(struct bpf_object *obj, struct bpf_program *prog,
 					 struct bpf_insn *insns, int insns_cnt,
 					 const char *license, __u32 kern_version,
@@ -6842,6 +6860,10 @@ static int bpf_object_load_prog_instance(struct bpf_object *obj, struct bpf_prog
 		goto retry_load;
 
 	ret = -errno;
+
+	/* post-process verifier log to improve error descriptions */
+	fixup_verifier_log(prog, log_buf, log_buf_size);
+
 	cp = libbpf_strerror_r(errno, errmsg, sizeof(errmsg));
 	pr_warn("prog '%s': BPF program load failed: %s\n", prog->name, cp);
 	pr_perm_msg(ret);
@@ -6857,6 +6879,128 @@ static int bpf_object_load_prog_instance(struct bpf_object *obj, struct bpf_prog
 	return ret;
 }
 
+static char *find_prev_line(char *buf, char *cur)
+{
+	char *p;
+
+	if (cur == buf) /* end of a log buf */
+		return NULL;
+
+	p = cur - 1;
+	while (p - 1 >= buf && *(p - 1) != '\n')
+		p--;
+
+	return p;
+}
+
+static void patch_log(char *buf, size_t buf_sz, size_t log_sz,
+		      char *orig, size_t orig_sz, const char *patch)
+{
+	/* size of the remaining log content to the right from the to-be-replaced part */
+	size_t rem_sz = (buf + log_sz) - (orig + orig_sz);
+	size_t patch_sz = strlen(patch);
+
+	if (patch_sz != orig_sz) {
+		/* If patch line(s) are longer than original piece of verifier log,
+		 * shift log contents by (patch_sz - orig_sz) bytes to the right
+		 * starting from after to-be-replaced part of the log.
+		 *
+		 * If patch line(s) are shorter than original piece of verifier log,
+		 * shift log contents by (orig_sz - patch_sz) bytes to the left
+		 * starting from after to-be-replaced part of the log
+		 *
+		 * We need to be careful about not overflowing available
+		 * buf_sz capacity. If that's the case, we'll truncate the end
+		 * of the original log, as necessary.
+		 */
+		if (patch_sz > orig_sz) {
+			if (orig + patch_sz >= buf + buf_sz) {
+				/* patch is big enough to cover remaining space completely */
+				patch_sz -= (orig + patch_sz) - (buf + buf_sz) + 1;
+				rem_sz = 0;
+			} else if (patch_sz - orig_sz > buf_sz - log_sz) {
+				/* patch causes part of remaining log to be truncated */
+				rem_sz -= (patch_sz - orig_sz) - (buf_sz - log_sz);
+			}
+		}
+		/* shift remaining log to the right by calculated amount */
+		memmove(orig + patch_sz, orig + orig_sz, rem_sz);
+	}
+
+	memcpy(orig, patch, patch_sz);
+}
+
+static void fixup_log_failed_core_relo(struct bpf_program *prog,
+				       char *buf, size_t buf_sz, size_t log_sz,
+				       char *line1, char *line2, char *line3)
+{
+	/* Expected log for failed and not properly guarded CO-RE relocation:
+	 * line1 -> 123: (85) call unknown#195896080
+	 * line2 -> invalid func unknown#195896080
+	 * line3 -> <anything else or end of buffer>
+	 *
+	 * "123" is the index of the instruction that was poisoned. We extract
+	 * instruction index to find corresponding CO-RE relocation and
+	 * replace this part of the log with more relevant information about
+	 * failed CO-RE relocation.
+	 */
+	const struct bpf_core_relo *relo;
+	struct bpf_core_spec spec;
+	char patch[512], spec_buf[256];
+	int insn_idx, err;
+
+	if (sscanf(line1, "%d: (%*d) call unknown#195896080\n", &insn_idx) != 1)
+		return;
+
+	relo = find_relo_core(prog, insn_idx);
+	if (!relo)
+		return;
+
+	err = bpf_core_parse_spec(prog->name, prog->obj->btf, relo, &spec);
+	if (err)
+		return;
+
+	bpf_core_format_spec(spec_buf, sizeof(spec_buf), &spec);
+	snprintf(patch, sizeof(patch),
+		 "%d: <invalid CO-RE relocation>\n"
+		 "failed to resolve CO-RE relocation %s\n",
+		 insn_idx, spec_buf);
+
+	patch_log(buf, buf_sz, log_sz, line1, line3 - line1, patch);
+}
+
+static void fixup_verifier_log(struct bpf_program *prog, char *buf, size_t buf_sz)
+{
+	/* look for familiar error patterns in last N lines of the log */
+	const size_t max_last_line_cnt = 10;
+	char *prev_line, *cur_line, *next_line;
+	size_t log_sz;
+	int i;
+
+	if (!buf)
+		return;
+
+	log_sz = strlen(buf) + 1;
+	next_line = buf + log_sz - 1;
+
+	for (i = 0; i < max_last_line_cnt; i++, next_line = cur_line) {
+		cur_line = find_prev_line(buf, next_line);
+		if (!cur_line)
+			return;
+
+		/* failed CO-RE relocation case */
+		if (str_has_pfx(cur_line, "invalid func unknown#195896080\n")) {
+			prev_line = find_prev_line(buf, cur_line);
+			if (!prev_line)
+				continue;
+
+			fixup_log_failed_core_relo(prog, buf, buf_sz, log_sz,
+						   prev_line, cur_line, next_line);
+			return;
+		}
+	}
+}
+
 static int bpf_program_record_relos(struct bpf_program *prog)
 {
 	struct bpf_object *obj = prog->obj;
diff --git a/tools/lib/bpf/relo_core.c b/tools/lib/bpf/relo_core.c
index 4a9ad0cfb474..ba4453dfd1ed 100644
--- a/tools/lib/bpf/relo_core.c
+++ b/tools/lib/bpf/relo_core.c
@@ -178,9 +178,9 @@ static bool core_relo_is_enumval_based(enum bpf_core_relo_kind kind)
  * Enum value-based relocations (ENUMVAL_EXISTS/ENUMVAL_VALUE) use access
  * string to specify enumerator's value index that need to be relocated.
  */
-static int bpf_core_parse_spec(const char *prog_name, const struct btf *btf,
-			       const struct bpf_core_relo *relo,
-			       struct bpf_core_spec *spec)
+int bpf_core_parse_spec(const char *prog_name, const struct btf *btf,
+			const struct bpf_core_relo *relo,
+			struct bpf_core_spec *spec)
 {
 	int access_idx, parsed_len, i;
 	struct bpf_core_accessor *acc;
@@ -1054,7 +1054,7 @@ int bpf_core_patch_insn(const char *prog_name, struct bpf_insn *insn,
  * [<type-id>] (<type-name>) + <raw-spec> => <offset>@<spec>,
  * where <spec> is a C-syntax view of recorded field access, e.g.: x.a[3].b
  */
-static int bpf_core_format_spec(char *buf, size_t buf_sz, const struct bpf_core_spec *spec)
+int bpf_core_format_spec(char *buf, size_t buf_sz, const struct bpf_core_spec *spec)
 {
 	const struct btf_type *t;
 	const struct btf_enum *e;
diff --git a/tools/lib/bpf/relo_core.h b/tools/lib/bpf/relo_core.h
index a28bf3711ce2..073039d8ca4f 100644
--- a/tools/lib/bpf/relo_core.h
+++ b/tools/lib/bpf/relo_core.h
@@ -84,4 +84,10 @@ int bpf_core_patch_insn(const char *prog_name, struct bpf_insn *insn,
 			int insn_idx, const struct bpf_core_relo *relo,
 			int relo_idx, const struct bpf_core_relo_res *res);
 
+int bpf_core_parse_spec(const char *prog_name, const struct btf *btf,
+		        const struct bpf_core_relo *relo,
+		        struct bpf_core_spec *spec);
+
+int bpf_core_format_spec(char *buf, size_t buf_sz, const struct bpf_core_spec *spec);
+
 #endif
-- 
2.30.2

Re: [PATCH bpf-next 09/10] libbpf: fix up verifier log for unguarded failed CO-RE relos

[Alexei Starovoitov]

On Mon, Apr 25, 2022 at 05:45:10PM -0700, Andrii Nakryiko wrote:
> Teach libbpf to post-process BPF verifier log on BPF program load
> failure and detect known error patterns to provide user with more
> context.
> 
> Currently there is one such common situation: an "unguarded" failed BPF
> CO-RE relocation. While failing CO-RE relocation is expected, it is
> expected to be property guarded in BPF code such that BPF verifier
> always eliminates BPF instructions corresponding to such failed CO-RE
> relos as dead code. In cases when user failed to take such precautions,
> BPF verifier provides the best log it can:
> 
>   123: (85) call unknown#195896080
>   invalid func unknown#195896080
> 
> Such incomprehensible log error is due to libbpf "poisoning" BPF
> instruction that corresponds to failed CO-RE relocation by replacing it
> with invalid `call 0xbad2310` instruction (195896080 == 0xbad2310 reads
> "bad relo" if you squint hard enough).
> 
> Luckily, libbpf has all the necessary information to look up CO-RE
> relocation that failed and provide more human-readable description of
> what's going on:
> 
>   5: <invalid CO-RE relocation>
>   failed to resolve CO-RE relocation <byte_off> [6] struct task_struct___bad.fake_field_subprog (0:2 @ offset 8)
> 
> This hopefully makes it much easier to understand what's wrong with
> user's BPF program without googling magic constants.
> 
> This BPF verifier log fixup is setup to be extensible and is going to be
> used for at least one other upcoming feature of libbpf in follow up patches.
> Libbpf is parsing lines of BPF verifier log starting from the very end.
> Currently it processes up to 10 lines of code looking for familiar
> patterns. This avoids wasting lots of CPU processing huge verifier logs
> (especially for log_level=2 verbosity level). Actual verification error
> should normally be found in last few lines, so this should work
> reliably.
> 
> If libbpf needs to expand log beyond available log_buf_size, it
> truncates the end of the verifier log. Given verifier log normally ends
> with something like:
> 
>   processed 2 insns (limit 1000000) max_states_per_insn 0 total_states 0 peak_states 0 mark_read 0
> 
> ... truncating this on program load error isn't too bad (end user can
> always increase log size, if it needs to get complete log).

and it didn't break test_verifier?
In do_test_single() it does:
  proc = strstr(bpf_vlog, "processed ");
  insn_processed = atoi(proc + 10);
  if (test->insn_processed != insn_processed) {

Re: [PATCH bpf-next 09/10] libbpf: fix up verifier log for unguarded failed CO-RE relos

[Andrii Nakryiko]

On Tue, Apr 26, 2022 at 11:59 AM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Mon, Apr 25, 2022 at 05:45:10PM -0700, Andrii Nakryiko wrote:
> > Teach libbpf to post-process BPF verifier log on BPF program load
> > failure and detect known error patterns to provide user with more
> > context.
> >
> > Currently there is one such common situation: an "unguarded" failed BPF
> > CO-RE relocation. While failing CO-RE relocation is expected, it is
> > expected to be property guarded in BPF code such that BPF verifier
> > always eliminates BPF instructions corresponding to such failed CO-RE
> > relos as dead code. In cases when user failed to take such precautions,
> > BPF verifier provides the best log it can:
> >
> >   123: (85) call unknown#195896080
> >   invalid func unknown#195896080
> >
> > Such incomprehensible log error is due to libbpf "poisoning" BPF
> > instruction that corresponds to failed CO-RE relocation by replacing it
> > with invalid `call 0xbad2310` instruction (195896080 == 0xbad2310 reads
> > "bad relo" if you squint hard enough).
> >
> > Luckily, libbpf has all the necessary information to look up CO-RE
> > relocation that failed and provide more human-readable description of
> > what's going on:
> >
> >   5: <invalid CO-RE relocation>
> >   failed to resolve CO-RE relocation <byte_off> [6] struct task_struct___bad.fake_field_subprog (0:2 @ offset 8)
> >
> > This hopefully makes it much easier to understand what's wrong with
> > user's BPF program without googling magic constants.
> >
> > This BPF verifier log fixup is setup to be extensible and is going to be
> > used for at least one other upcoming feature of libbpf in follow up patches.
> > Libbpf is parsing lines of BPF verifier log starting from the very end.
> > Currently it processes up to 10 lines of code looking for familiar
> > patterns. This avoids wasting lots of CPU processing huge verifier logs
> > (especially for log_level=2 verbosity level). Actual verification error
> > should normally be found in last few lines, so this should work
> > reliably.
> >
> > If libbpf needs to expand log beyond available log_buf_size, it
> > truncates the end of the verifier log. Given verifier log normally ends
> > with something like:
> >
> >   processed 2 insns (limit 1000000) max_states_per_insn 0 total_states 0 peak_states 0 mark_read 0
> >
> > ... truncating this on program load error isn't too bad (end user can
> > always increase log size, if it needs to get complete log).
>
> and it didn't break test_verifier?
> In do_test_single() it does:
>   proc = strstr(bpf_vlog, "processed ");
>   insn_processed = atoi(proc + 10);
>   if (test->insn_processed != insn_processed) {

I forgot to check test_verifier locally, but it's fine according to CI
([0]). This truncation can only happen if libbpf fixes up verifier
log, which currently happens only when there is CO-RE relocation
failure. I don't think we have any CO-RE relocation failure tests in
test_verifier itself. For all other case there will be absolutely no
change in verifier log output.

  [0] https://github.com/kernel-patches/bpf/runs/6181657272?check_suite_focus=true

[PATCH bpf-next 10/10] selftests/bpf: add libbpf's log fixup logic selftests

[Andrii Nakryiko]

Add tests validating that libbpf is indeed patching up BPF verifier log
with CO-RE relocation details. Also test partial and full truncation
scenarios.

This test might be a bit fragile due to changing BPF verifier log
format. If that proves to be frequently breaking, we can simplify tests
or remove the truncation subtests. But for now it seems useful to test
it in those conditions that are otherwise rarely occuring in practice.

Also test CO-RE relo failure in a subprog as that excercises subprogram CO-RE
relocation mapping logic which doesn't work out of the box without extra
relo storage previously done only for gen_loader case.

Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
 .../selftests/bpf/prog_tests/log_fixup.c      | 114 ++++++++++++++++++
 .../selftests/bpf/progs/test_log_fixup.c      |  38 ++++++
 tools/testing/selftests/bpf/test_progs.h      |  11 ++
 3 files changed, 163 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/log_fixup.c
 create mode 100644 tools/testing/selftests/bpf/progs/test_log_fixup.c

diff --git a/tools/testing/selftests/bpf/prog_tests/log_fixup.c b/tools/testing/selftests/bpf/prog_tests/log_fixup.c
new file mode 100644
index 000000000000..be3a956cb3a5
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/log_fixup.c
@@ -0,0 +1,114 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2022 Meta Platforms, Inc. and affiliates. */
+#include <test_progs.h>
+#include <bpf/btf.h>
+
+#include "test_log_fixup.skel.h"
+
+enum trunc_type {
+	TRUNC_NONE,
+	TRUNC_PARTIAL,
+	TRUNC_FULL,
+};
+
+static void bad_core_relo(size_t log_buf_size, enum trunc_type trunc_type)
+{
+	char log_buf[8 * 1024];
+	struct test_log_fixup* skel;
+	int err;
+
+	skel = test_log_fixup__open();
+	if (!ASSERT_OK_PTR(skel, "skel_open"))
+		return;
+
+	bpf_program__set_autoload(skel->progs.bad_relo, true);
+	memset(log_buf, 0, sizeof(log_buf));
+	bpf_program__set_log_buf(skel->progs.bad_relo, log_buf, log_buf_size ?: sizeof(log_buf));
+
+	err = test_log_fixup__load(skel);
+	if (!ASSERT_ERR(err, "load_fail"))
+		goto cleanup;
+
+	ASSERT_HAS_SUBSTR(log_buf,
+			  "0: <invalid CO-RE relocation>\n"
+			  "failed to resolve CO-RE relocation <byte_sz> ",
+			  "log_buf_part1");
+
+	switch (trunc_type) {
+	case TRUNC_NONE:
+		ASSERT_HAS_SUBSTR(log_buf,
+				  "struct task_struct___bad.fake_field (0:1 @ offset 4)\n",
+				  "log_buf_part2");
+		ASSERT_HAS_SUBSTR(log_buf,
+				  "max_states_per_insn 0 total_states 0 peak_states 0 mark_read 0\n",
+				  "log_buf_end");
+		break;
+	case TRUNC_PARTIAL:
+		/* we should get full libbpf message patch */
+		ASSERT_HAS_SUBSTR(log_buf,
+				  "struct task_struct___bad.fake_field (0:1 @ offset 4)\n",
+				  "log_buf_part2");
+		/* we shouldn't get full end of BPF verifier log */
+		ASSERT_NULL(strstr(log_buf, "max_states_per_insn 0 total_states 0 peak_states 0 mark_read 0\n"),
+			    "log_buf_end");
+		break;
+	case TRUNC_FULL:
+		/* we shouldn't get second part of libbpf message patch */
+		ASSERT_NULL(strstr(log_buf, "struct task_struct___bad.fake_field (0:1 @ offset 4)\n"),
+			    "log_buf_part2");
+		/* we shouldn't get full end of BPF verifier log */
+		ASSERT_NULL(strstr(log_buf, "max_states_per_insn 0 total_states 0 peak_states 0 mark_read 0\n"),
+			    "log_buf_end");
+		break;
+	}
+
+	if (env.verbosity > VERBOSE_NONE)
+		printf("LOG:   \n=================\n%s=================\n", log_buf);
+cleanup:
+	test_log_fixup__destroy(skel);
+}
+
+static void bad_core_relo_subprog(void)
+{
+	char log_buf[8 * 1024];
+	struct test_log_fixup* skel;
+	int err;
+
+	skel = test_log_fixup__open();
+	if (!ASSERT_OK_PTR(skel, "skel_open"))
+		return;
+
+	bpf_program__set_autoload(skel->progs.bad_relo_subprog, true);
+	bpf_program__set_log_buf(skel->progs.bad_relo_subprog, log_buf, sizeof(log_buf));
+
+	err = test_log_fixup__load(skel);
+	if (!ASSERT_ERR(err, "load_fail"))
+		goto cleanup;
+
+	/* there should be no prog loading log because we specified per-prog log buf */
+	ASSERT_HAS_SUBSTR(log_buf,
+			  ": <invalid CO-RE relocation>\n"
+			  "failed to resolve CO-RE relocation <byte_off> ",
+			  "log_buf");
+	ASSERT_HAS_SUBSTR(log_buf,
+			  "struct task_struct___bad.fake_field_subprog (0:2 @ offset 8)\n",
+			  "log_buf");
+
+	if (env.verbosity > VERBOSE_NONE)
+		printf("LOG:   \n=================\n%s=================\n", log_buf);
+
+cleanup:
+	test_log_fixup__destroy(skel);
+}
+
+void test_log_fixup(void)
+{
+	if (test__start_subtest("bad_core_relo_trunc_none"))
+		bad_core_relo(0, TRUNC_NONE /* full buf */);
+	if (test__start_subtest("bad_core_relo_trunc_partial"))
+		bad_core_relo(300, TRUNC_PARTIAL /* truncate original log a bit */);
+	if (test__start_subtest("bad_core_relo_trunc_full"))
+		bad_core_relo(250, TRUNC_FULL  /* truncate also libbpf's message patch */);
+	if (test__start_subtest("bad_core_relo_subprog"))
+		bad_core_relo_subprog();
+}
diff --git a/tools/testing/selftests/bpf/progs/test_log_fixup.c b/tools/testing/selftests/bpf/progs/test_log_fixup.c
new file mode 100644
index 000000000000..a78980d897b3
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/test_log_fixup.c
@@ -0,0 +1,38 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2022 Meta Platforms, Inc. and affiliates. */
+
+#include <linux/bpf.h>
+#include <bpf/bpf_helpers.h>
+#include <bpf/bpf_core_read.h>
+
+struct task_struct___bad {
+	int pid;
+	int fake_field;
+	void *fake_field_subprog;
+} __attribute__((preserve_access_index));
+
+SEC("?raw_tp/sys_enter")
+int bad_relo(const void *ctx)
+{
+	static struct task_struct___bad *t;
+
+	return bpf_core_field_size(t->fake_field);
+}
+
+static __noinline int bad_subprog(void)
+{
+	static struct task_struct___bad *t;
+
+	/* ugliness below is a field offset relocation */
+	return (void *)&t->fake_field_subprog - (void *)t;
+}
+
+SEC("?raw_tp/sys_enter")
+int bad_relo_subprog(const void *ctx)
+{
+	static struct task_struct___bad *t;
+
+	return bad_subprog() + bpf_core_field_size(t->pid);
+}
+
+char _license[] SEC("license") = "GPL";
diff --git a/tools/testing/selftests/bpf/test_progs.h b/tools/testing/selftests/bpf/test_progs.h
index 0a102ce460d6..d3fee3b98888 100644
--- a/tools/testing/selftests/bpf/test_progs.h
+++ b/tools/testing/selftests/bpf/test_progs.h
@@ -292,6 +292,17 @@ int test__join_cgroup(const char *path);
 	___ok;								\
 })
 
+#define ASSERT_HAS_SUBSTR(str, substr, name) ({				\
+	static int duration = 0;					\
+	const char *___str = str;					\
+	const char *___substr = substr;					\
+	bool ___ok = strstr(___str, ___substr) != NULL;			\
+	CHECK(!___ok, (name),						\
+	      "unexpected %s: '%s' is not a substring of '%s'\n",	\
+	      (name), ___substr, ___str);				\
+	___ok;								\
+})
+
 #define ASSERT_OK(res, name) ({						\
 	static int duration = 0;					\
 	long long ___res = (res);					\
-- 
2.30.2

Re: [PATCH bpf-next 00/10] Teach libbpf to "fix up" BPF verifier log

[patchwork-bot+netdevbpf]

Hello:

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

On Mon, 25 Apr 2022 17:45:01 -0700 you wrote:
> This patch set teaches libbpf to enhance BPF verifier log with human-readable
> and relevant information about failed CO-RE relocation. Patch #9 is the main
> one with the new logic. See relevant commit messages for some more details.
> 
> All the other patches are either fixing various bugs detected
> while working on this feature, most prominently a bug with libbpf not handling
> CO-RE relocations for SEC("?...") programs, or are refactoring libbpf
> internals to allow for easier reuse of CO-RE relo lookup and formatting logic.
> 
> [...]

Here is the summary with links:
  - [bpf-next,01/10] libbpf: fix anonymous type check in CO-RE logic
    https://git.kernel.org/bpf/bpf-next/c/afe98d46ba22
  - [bpf-next,02/10] libbpf: drop unhelpful "program too large" guess
    https://git.kernel.org/bpf/bpf-next/c/0994a54c5202
  - [bpf-next,03/10] libbpf: fix logic for finding matching program for CO-RE relocation
    https://git.kernel.org/bpf/bpf-next/c/966a75093253
  - [bpf-next,04/10] libbpf: avoid joining .BTF.ext data with BPF programs by section name
    https://git.kernel.org/bpf/bpf-next/c/11d5daa89254
  - [bpf-next,05/10] selftests/bpf: add CO-RE relos and SEC("?...") to linked_funcs selftests
    https://git.kernel.org/bpf/bpf-next/c/b82bb1ffbb9a
  - [bpf-next,06/10] libbpf: record subprog-resolved CO-RE relocations unconditionally
    https://git.kernel.org/bpf/bpf-next/c/185cfe837fdb
  - [bpf-next,07/10] libbpf: refactor CO-RE relo human description formatting routine
    https://git.kernel.org/bpf/bpf-next/c/b58af63aab11
  - [bpf-next,08/10] libbpf: simplify bpf_core_parse_spec() signature
    https://git.kernel.org/bpf/bpf-next/c/14032f264453
  - [bpf-next,09/10] libbpf: fix up verifier log for unguarded failed CO-RE relos
    https://git.kernel.org/bpf/bpf-next/c/9fdc4273b8da
  - [bpf-next,10/10] selftests/bpf: add libbpf's log fixup logic selftests
    https://git.kernel.org/bpf/bpf-next/c/ea4128eb43eb

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