[PATCH bpf-next v3 0/4] bpf: Improve error reporting for freplace attachment failure

[PATCH bpf-next v3 0/4] bpf: Improve error reporting for freplace attachment failure

[Leon Hwang]

This patch series improves error reporting for BPF_LINK_CREATE when
attaching freplace programs. Inspired by the discussion in
"[PATCH bpf-next v2] bpf: Add bpf_check_attach_target_with_klog method to
output failure logs to kernel"[0], this series enhances that freplace
attachment failure returns meaningful logs to userspace, aiding debugging.

For example, when attempting to attach a freplace program to a static
function:

libbpf: prog 'new_test_pkt_access': failed to attach to freplace: -EINVAL
libbpf: prog 'new_test_pkt_access': attach log: subprog_tail() is not a global function

Patch breakdown:
1. bpf, verifier: Add missing newline of bpf_log in bpf_check_attach_target
    * Add the missing newline in
      bpf_log(log, "Target program bound device mismatch").
2. bpf: Report log of freplace attach failure
    * Extends BPF_LINK_CREATE to report detailed error logs.
3. bpf, libbpf: Capture log of freplace attach failure
    * Modifies libbpf to capture and print attachment logs.
4. selftests/bpf: Add a test case for freplace attachment failure logging
    * Introduces a selftest to validate error reporting.

Links:
[0] https://lore.kernel.org/bpf/CAEf4BzbbyojuFSS7xQ3+jZb=dHzOaZfMbtT+WnypW2LPwOUwRw@mail.gmail.com/

Changes:
v2: https://lore.kernel.org/bpf/20240725051511.57112-1-me@manjusaka.me/
v2 -> v3:
  * Address comment from Andrii:
    * Report back the reason for declining freplace attachment instead of
      logging in dmesg.

Leon Hwang (4):
  bpf, verifier: Add missing newline of bpf_log in
    bpf_check_attach_target
  bpf: Improve error reporting for freplace attachment failure
  bpf, libbpf: Capture and log freplace attachment failure
  selftests/bpf: Add test case for freplace attachment failure logging

 include/uapi/linux/bpf.h                      |  2 +
 kernel/bpf/syscall.c                          | 51 ++++++++++++++++---
 kernel/bpf/verifier.c                         |  2 +-
 tools/include/uapi/linux/bpf.h                |  2 +
 tools/lib/bpf/bpf.c                           |  6 ++-
 tools/lib/bpf/bpf.h                           |  2 +
 tools/lib/bpf/libbpf.c                        | 14 ++++-
 .../bpf/prog_tests/tracing_link_attach_log.c  | 42 +++++++++++++++
 8 files changed, 109 insertions(+), 12 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/tracing_link_attach_log.c

-- 
2.47.1

[PATCH bpf-next v3 1/4] bpf, verifier: Add missing newline of bpf_log in bpf_check_attach_target

[Leon Hwang]

Add the missing newline in
bpf_log(log, "Target program bound device mismatch").

Signed-off-by: Leon Hwang <leon.hwang@linux.dev>
---
 kernel/bpf/verifier.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 3c286ec9e8413..2f9fc84f9b1a7 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -22574,7 +22574,7 @@ int bpf_check_attach_target(struct bpf_verifier_log *log,
 
 		if (bpf_prog_is_dev_bound(prog->aux) &&
 		    !bpf_prog_dev_bound_match(prog, tgt_prog)) {
-			bpf_log(log, "Target program bound device mismatch");
+			bpf_log(log, "Target program bound device mismatch\n");
 			return -EINVAL;
 		}
 
-- 
2.47.1

[PATCH bpf-next v3 2/4] bpf: Improve error reporting for freplace attachment failure

[Leon Hwang]

When a freplace program fails to attach to a target, the error message
lacks details, making debugging difficult. This patch enhances error
reporting by providing a log that explains why the attachment failed.

For example, if a freplace program tries to attach to a static function,
the log now includes:

libbpf: prog 'new_test_pkt_access': failed to attach to freplace: -EINVAL
libbpf: prog 'new_test_pkt_access': attach log: subprog_tail() is not a global function

Changes:

* Added verifier log to capture the log of freplace attachment failure.
* Updated bpf_tracing_prog_attach() to accept verifier log.
* Extended struct bpf_attr with a user-supplied log buffer for tracing
  programs.

This improves debugging by giving clear feedback when a freplace
attachment fails.

Signed-off-by: Leon Hwang <leon.hwang@linux.dev>
---
 include/uapi/linux/bpf.h |  2 ++
 kernel/bpf/syscall.c     | 51 +++++++++++++++++++++++++++++++++-------
 2 files changed, 45 insertions(+), 8 deletions(-)

diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index fff6cdb8d11a2..bea4d802d4463 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -1759,6 +1759,8 @@ union bpf_attr {
 				 * accessible through bpf_get_attach_cookie() BPF helper
 				 */
 				__u64		cookie;
+				__aligned_u64	log_buf;	/* user supplied buffer */
+				__u32		log_size;	/* size of user buffer */
 			} tracing;
 			struct {
 				__u32		pf;
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index c420edbfb7c87..f41d1eea102a4 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -3412,7 +3412,8 @@ static const struct bpf_link_ops bpf_tracing_link_lops = {
 static int bpf_tracing_prog_attach(struct bpf_prog *prog,
 				   int tgt_prog_fd,
 				   u32 btf_id,
-				   u64 bpf_cookie)
+				   u64 bpf_cookie,
+				   struct bpf_verifier_log *log)
 {
 	struct bpf_link_primer link_primer;
 	struct bpf_prog *tgt_prog = NULL;
@@ -3537,7 +3538,7 @@ static int bpf_tracing_prog_attach(struct bpf_prog *prog,
 		 */
 		struct bpf_attach_target_info tgt_info = {};
 
-		err = bpf_check_attach_target(NULL, prog, tgt_prog, btf_id,
+		err = bpf_check_attach_target(log, prog, tgt_prog, btf_id,
 					      &tgt_info);
 		if (err)
 			goto out_unlock;
@@ -3949,7 +3950,7 @@ static int bpf_raw_tp_link_attach(struct bpf_prog *prog,
 			tp_name = prog->aux->attach_func_name;
 			break;
 		}
-		return bpf_tracing_prog_attach(prog, 0, 0, 0);
+		return bpf_tracing_prog_attach(prog, 0, 0, 0, NULL);
 	case BPF_PROG_TYPE_RAW_TRACEPOINT:
 	case BPF_PROG_TYPE_RAW_TRACEPOINT_WRITABLE:
 		if (strncpy_from_user(buf, user_tp_name, sizeof(buf) - 1) < 0)
@@ -5311,9 +5312,13 @@ static int bpf_map_do_batch(const union bpf_attr *attr,
 }
 
 #define BPF_LINK_CREATE_LAST_FIELD link_create.uprobe_multi.pid
-static int link_create(union bpf_attr *attr, bpfptr_t uattr)
+static int link_create(union bpf_attr *attr, bpfptr_t uattr, u32 uattr_size)
 {
+	struct bpf_verifier_log *log;
+	u32 log_true_size, log_size;
 	struct bpf_prog *prog;
+	__aligned_u64 log_buf;
+	bool use_log;
 	int ret;
 
 	if (CHECK_ATTR(BPF_LINK_CREATE))
@@ -5326,10 +5331,33 @@ static int link_create(union bpf_attr *attr, bpfptr_t uattr)
 	if (IS_ERR(prog))
 		return PTR_ERR(prog);
 
+	switch (prog->type) {
+	case BPF_PROG_TYPE_EXT:
+		log_buf = attr->link_create.tracing.log_buf;
+		log_size = attr->link_create.tracing.log_size;
+		use_log = true;
+		break;
+	default:
+		use_log = false;
+	}
+
+	if (use_log) {
+		log = kvzalloc(sizeof(*log), GFP_KERNEL);
+		if (!log) {
+			ret = -ENOMEM;
+			goto out;
+		}
+		ret = bpf_vlog_init(log, BPF_LOG_FIXED,
+				    (char __user *) (unsigned long) log_buf,
+				    log_size);
+		if (ret)
+			goto out_free_log;
+	}
+
 	ret = bpf_prog_attach_check_attach_type(prog,
 						attr->link_create.attach_type);
 	if (ret)
-		goto out;
+		goto out_free_log;
 
 	switch (prog->type) {
 	case BPF_PROG_TYPE_CGROUP_SKB:
@@ -5345,7 +5373,8 @@ static int link_create(union bpf_attr *attr, bpfptr_t uattr)
 		ret = bpf_tracing_prog_attach(prog,
 					      attr->link_create.target_fd,
 					      attr->link_create.target_btf_id,
-					      attr->link_create.tracing.cookie);
+					      attr->link_create.tracing.cookie,
+					      log);
 		break;
 	case BPF_PROG_TYPE_LSM:
 	case BPF_PROG_TYPE_TRACING:
@@ -5363,7 +5392,8 @@ static int link_create(union bpf_attr *attr, bpfptr_t uattr)
 			ret = bpf_tracing_prog_attach(prog,
 						      attr->link_create.target_fd,
 						      attr->link_create.target_btf_id,
-						      attr->link_create.tracing.cookie);
+						      attr->link_create.tracing.cookie,
+						      NULL);
 		break;
 	case BPF_PROG_TYPE_FLOW_DISSECTOR:
 	case BPF_PROG_TYPE_SK_LOOKUP:
@@ -5406,6 +5436,11 @@ static int link_create(union bpf_attr *attr, bpfptr_t uattr)
 		ret = -EINVAL;
 	}
 
+	if (ret < 0 && use_log)
+		(void) bpf_vlog_finalize(log, &log_true_size);
+out_free_log:
+	if (use_log)
+		kvfree(log);
 out:
 	if (ret < 0)
 		bpf_prog_put(prog);
@@ -5861,7 +5896,7 @@ static int __sys_bpf(enum bpf_cmd cmd, bpfptr_t uattr, unsigned int size)
 		err = bpf_map_do_batch(&attr, uattr.user, BPF_MAP_DELETE_BATCH);
 		break;
 	case BPF_LINK_CREATE:
-		err = link_create(&attr, uattr);
+		err = link_create(&attr, uattr, size);
 		break;
 	case BPF_LINK_UPDATE:
 		err = link_update(&attr);
-- 
2.47.1

[PATCH bpf-next v3 3/4] bpf, libbpf: Capture and log freplace attachment failure

[Leon Hwang]

To improve debugging, this patch captures logs when a freplace program
fails to attach. It provides a buffer to store the log and prints it using
pr_warn, making failure reasons more visible.

Changes:

* Extended bpf_attr and bpf_link_create_opts to include a log buffer for
  tracing.
* Updated bpf_link_create() to handle log buffer properly.
* Modified bpf_program__attach_freplace() to store and print attachment
  failure log.

Example output:

prog 'new_test_pkt_access': attach log: subprog_tail() is not a global function

This helps diagnose freplace attachment failures more efficiently.

Signed-off-by: Leon Hwang <leon.hwang@linux.dev>
---
 tools/include/uapi/linux/bpf.h |  2 ++
 tools/lib/bpf/bpf.c            |  6 +++++-
 tools/lib/bpf/bpf.h            |  2 ++
 tools/lib/bpf/libbpf.c         | 14 ++++++++++++--
 4 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index fff6cdb8d11a2..bea4d802d4463 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -1759,6 +1759,8 @@ union bpf_attr {
 				 * accessible through bpf_get_attach_cookie() BPF helper
 				 */
 				__u64		cookie;
+				__aligned_u64	log_buf;	/* user supplied buffer */
+				__u32		log_size;	/* size of user buffer */
 			} tracing;
 			struct {
 				__u32		pf;
diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c
index 359f73ead6137..cd422ecd53ae2 100644
--- a/tools/lib/bpf/bpf.c
+++ b/tools/lib/bpf/bpf.c
@@ -741,7 +741,7 @@ int bpf_link_create(int prog_fd, int target_fd,
 	if (iter_info_len || target_btf_id) {
 		if (iter_info_len && target_btf_id)
 			return libbpf_err(-EINVAL);
-		if (!OPTS_ZEROED(opts, target_btf_id))
+		if (!OPTS_ZEROED(opts, tracing))
 			return libbpf_err(-EINVAL);
 	}
 
@@ -753,6 +753,8 @@ int bpf_link_create(int prog_fd, int target_fd,
 
 	if (target_btf_id) {
 		attr.link_create.target_btf_id = target_btf_id;
+		attr.link_create.tracing.log_buf = ptr_to_u64(OPTS_GET(opts, tracing.log_buf, 0));
+		attr.link_create.tracing.log_size = OPTS_GET(opts, tracing.log_size, 0);
 		goto proceed;
 	}
 
@@ -794,6 +796,8 @@ int bpf_link_create(int prog_fd, int target_fd,
 	case BPF_MODIFY_RETURN:
 	case BPF_LSM_MAC:
 		attr.link_create.tracing.cookie = OPTS_GET(opts, tracing.cookie, 0);
+		attr.link_create.tracing.log_buf = 0;
+		attr.link_create.tracing.log_size = 0;
 		if (!OPTS_ZEROED(opts, tracing))
 			return libbpf_err(-EINVAL);
 		break;
diff --git a/tools/lib/bpf/bpf.h b/tools/lib/bpf/bpf.h
index 435da95d20589..daf62f1bda80f 100644
--- a/tools/lib/bpf/bpf.h
+++ b/tools/lib/bpf/bpf.h
@@ -421,6 +421,8 @@ struct bpf_link_create_opts {
 		} uprobe_multi;
 		struct {
 			__u64 cookie;
+			const char *log_buf;
+			__u32 log_size;
 		} tracing;
 		struct {
 			__u32 pf;
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 194809da51725..f9266bd0ff709 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -12841,6 +12841,8 @@ struct bpf_link *bpf_program__attach_freplace(const struct bpf_program *prog,
 					      int target_fd,
 					      const char *attach_func_name)
 {
+	struct bpf_link *link;
+	char log_buf[64];
 	int btf_id;
 
 	if (!!target_fd != !!attach_func_name) {
@@ -12862,10 +12864,18 @@ struct bpf_link *bpf_program__attach_freplace(const struct bpf_program *prog,
 		if (btf_id < 0)
 			return libbpf_err_ptr(btf_id);
 
+		log_buf[0] = '\0';
 		target_opts.target_btf_id = btf_id;
-
-		return bpf_program_attach_fd(prog, target_fd, "freplace",
+		target_opts.tracing.log_buf = log_buf;
+		target_opts.tracing.log_size = sizeof(log_buf);
+		link = bpf_program_attach_fd(prog, target_fd, "freplace",
 					     &target_opts);
+		if (libbpf_get_error(link) && log_buf[0] != '\0') {
+			log_buf[sizeof(log_buf)-1] = '\0';
+			log_buf[sizeof(log_buf)-2] = '\n';
+			pr_warn("prog '%s': attach log: %s", prog->name, log_buf);
+		}
+		return link;
 	} else {
 		/* no target, so use raw_tracepoint_open for compatibility
 		 * with old kernels
-- 
2.47.1

[PATCH bpf-next v3 4/4] selftests/bpf: Add test case for freplace attachment failure logging

[Leon Hwang]

This patch adds a selftest to verify that freplace attachment failure
produces meaningful logs.

cd tools/testing/selftests/bpf/; ./test_progs -t attach_log -v
test_freplace_attach_log:PASS:tailcall_bpf2bpf1__open_and_load 0 nsec
test_freplace_attach_log:PASS:freplace_global_func__open 0 nsec
test_freplace_attach_log:PASS:bpf_program__set_attach_target 0 nsec
test_freplace_attach_log:PASS:freplace_global_func__load 0 nsec
libbpf: prog 'new_test_pkt_access': failed to attach to freplace: -EINVAL
libbpf: prog 'new_test_pkt_access': attach log: subprog_tail() is not a global function
test_freplace_attach_log:PASS:bpf_program__attach_freplace 0 nsec
114     freplace_attach_log:OK
Summary: 1/0 PASSED, 0 SKIPPED, 0 FAILED

Signed-off-by: Leon Hwang <leon.hwang@linux.dev>
---
 .../bpf/prog_tests/tracing_link_attach_log.c  | 42 +++++++++++++++++++
 1 file changed, 42 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/tracing_link_attach_log.c

diff --git a/tools/testing/selftests/bpf/prog_tests/tracing_link_attach_log.c b/tools/testing/selftests/bpf/prog_tests/tracing_link_attach_log.c
new file mode 100644
index 0000000000000..cfdcb9ebdd255
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/tracing_link_attach_log.c
@@ -0,0 +1,42 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright Leon Hwang */
+
+#include <test_progs.h>
+#include "tailcall_bpf2bpf1.skel.h"
+#include "freplace_global_func.skel.h"
+
+void test_freplace_attach_log(void)
+{
+	struct freplace_global_func *freplace_skel = NULL;
+	struct tailcall_bpf2bpf1 *tailcall_skel = NULL;
+	struct bpf_link *freplace_link = NULL;
+	struct bpf_program *prog;
+	int err, prog_fd;
+	LIBBPF_OPTS(bpf_link_create_opts, link_opts);
+
+	tailcall_skel = tailcall_bpf2bpf1__open_and_load();
+	if (!ASSERT_OK_PTR(tailcall_skel, "tailcall_bpf2bpf1__open_and_load"))
+		return;
+
+	freplace_skel = freplace_global_func__open();
+	if (!ASSERT_OK_PTR(freplace_skel, "freplace_global_func__open"))
+		goto out;
+
+	prog = freplace_skel->progs.new_test_pkt_access;
+	prog_fd = bpf_program__fd(tailcall_skel->progs.entry);
+	err = bpf_program__set_attach_target(prog, prog_fd, "entry");
+	if (!ASSERT_OK(err, "bpf_program__set_attach_target"))
+		goto out;
+
+	err = freplace_global_func__load(freplace_skel);
+	if (!ASSERT_OK(err, "freplace_global_func__load"))
+		goto out;
+
+	freplace_link = bpf_program__attach_freplace(prog, prog_fd, "subprog_tail");
+	ASSERT_ERR_PTR(freplace_link, "bpf_program__attach_freplace");
+
+out:
+	bpf_link__destroy(freplace_link);
+	freplace_global_func__destroy(freplace_skel);
+	tailcall_bpf2bpf1__destroy(tailcall_skel);
+}
-- 
2.47.1

Re: [PATCH bpf-next v3 2/4] bpf: Improve error reporting for freplace attachment failure

[Alexei Starovoitov]

On Mon, Feb 17, 2025 at 7:44 AM Leon Hwang <leon.hwang@linux.dev> wrote:
>
> When a freplace program fails to attach to a target, the error message
> lacks details, making debugging difficult. This patch enhances error
> reporting by providing a log that explains why the attachment failed.

Agree that it lacks details...

> For example, if a freplace program tries to attach to a static function,
> the log now includes:
>
> libbpf: prog 'new_test_pkt_access': failed to attach to freplace: -EINVAL
> libbpf: prog 'new_test_pkt_access': attach log: subprog_tail() is not a global function

... but adding to uapi for a minor usability improvement...
not a long term path that is worth taking.
Especially since freplace is special. Users don't interact with it
directly. The interaction is typically done through libraries.
So this extra verbosity won't help users directly, but will
help people who write libraries. Nice, but no.

Re: [PATCH bpf-next v3 2/4] bpf: Improve error reporting for freplace attachment failure

[Leon Hwang]

On 18/2/25 11:01, Alexei Starovoitov wrote:
> On Mon, Feb 17, 2025 at 7:44 AM Leon Hwang <leon.hwang@linux.dev> wrote:
>>
>> When a freplace program fails to attach to a target, the error message
>> lacks details, making debugging difficult. This patch enhances error
>> reporting by providing a log that explains why the attachment failed.
> 
> Agree that it lacks details...
> 
>> For example, if a freplace program tries to attach to a static function,
>> the log now includes:
>>
>> libbpf: prog 'new_test_pkt_access': failed to attach to freplace: -EINVAL
>> libbpf: prog 'new_test_pkt_access': attach log: subprog_tail() is not a global function
> 
> ... but adding to uapi for a minor usability improvement...
> not a long term path that is worth taking.
> Especially since freplace is special. Users don't interact with it
> directly. The interaction is typically done through libraries.
> So this extra verbosity won't help users directly, but will
> help people who write libraries. Nice, but no.

What if libbpf allows users to interact with it directly?

As for cilium/ebpf library, it will be able to provide the error message
to users like:

create link: subprog_tail() is not a global function

instead of:

create link: invalid argument

Thanks,
Leon