[PATCH bpf-next v9 0/4] Relax tracing prog recursive attach rules

[PATCH bpf-next v9 0/4] Relax tracing prog recursive attach rules

[Dmitrii Dolgov]

Currently, it's not allowed to attach an fentry/fexit prog to another
fentry/fexit. At the same time it's not uncommon to see a tracing
program with lots of logic in use, and the attachment limitation
prevents usage of fentry/fexit for performance analysis (e.g. with
"bpftool prog profile" command) in this case. An example could be
falcosecurity libs project that uses tp_btf tracing programs for
offloading certain part of logic into tail-called programs, but the
use-case is still generic enough -- a tracing program could be
complicated and heavy enough to warrant its profiling, yet frustratingly
it's not possible to do so use best tooling for that.

Following the corresponding discussion [1], the reason for that is to
avoid tracing progs call cycles without introducing more complex
solutions. But currently it seems impossible to load and attach tracing
programs in a way that will form such a cycle. Replace "no same type"
requirement with verification that no more than one level of attachment
nesting is allowed. In this way only one fentry/fexit program could be
attached to another fentry/fexit to cover profiling use case, and still
no cycle could be formed.

The series contains a test for recursive attachment, as well as a fix +
test for an issue in re-attachment branch of bpf_tracing_prog_attach.
When preparing the test for the main change set, I've stumbled upon the
possibility to construct a sequence of events when attach_btf would be
NULL while computing a trampoline key. It doesn't look like this issue
is triggered by the main change, because the reproduces doesn't actually
need to have an fentry attachment chain.

[1]: https://lore.kernel.org/bpf/20191108064039.2041889-16-ast@kernel.org/

Dmitrii Dolgov (3):
  bpf: Relax tracing prog recursive attach rules
  selftests/bpf: Add test for recursive attachment of tracing progs
  selftests/bpf: Test re-attachment fix for bpf_tracing_prog_attach

Jiri Olsa (1):
  bpf: Fix re-attachment branch in bpf_tracing_prog_attach

 include/linux/bpf.h                           |   1 +
 kernel/bpf/syscall.c                          |  19 ++-
 kernel/bpf/verifier.c                         |  39 +++---
 .../bpf/prog_tests/recursive_attach.c         | 113 ++++++++++++++++++
 .../selftests/bpf/progs/fentry_recursive.c    |  16 +++
 .../bpf/progs/fentry_recursive_target.c       |  27 +++++
 6 files changed, 200 insertions(+), 15 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/recursive_attach.c
 create mode 100644 tools/testing/selftests/bpf/progs/fentry_recursive.c
 create mode 100644 tools/testing/selftests/bpf/progs/fentry_recursive_target.c


base-commit: 40d0eb0259ae77ace3e81d7454d1068c38bc95c2
-- 
2.41.0

[PATCH bpf-next v9 1/4] bpf: Relax tracing prog recursive attach rules

[Dmitrii Dolgov]

Currently, it's not allowed to attach an fentry/fexit prog to another
one fentry/fexit. At the same time it's not uncommon to see a tracing
program with lots of logic in use, and the attachment limitation
prevents usage of fentry/fexit for performance analysis (e.g. with
"bpftool prog profile" command) in this case. An example could be
falcosecurity libs project that uses tp_btf tracing programs.

Following the corresponding discussion [1], the reason for that is to
avoid tracing progs call cycles without introducing more complex
solutions. But currently it seems impossible to load and attach tracing
programs in a way that will form such a cycle. The limitation is coming
from the fact that attach_prog_fd is specified at the prog load (thus
making it impossible to attach to a program loaded after it in this
way), as well as tracing progs not implementing link_detach.

Replace "no same type" requirement with verification that no more than
one level of attachment nesting is allowed. In this way only one
fentry/fexit program could be attached to another fentry/fexit to cover
profiling use case, and still no cycle could be formed. To implement,
add a new field into bpf_prog_aux to track nested attachment for tracing
programs.

[1]: https://lore.kernel.org/bpf/20191108064039.2041889-16-ast@kernel.org/

Signed-off-by: Dmitrii Dolgov <9erthalion6@gmail.com>
---
Previous discussion: https://lore.kernel.org/bpf/20231212195413.23942-1-9erthalion6@gmail.com/

Changes in v9:
    - Formatting

Changes in v8:
    - Move bookkeping in bpf_tracing_link_release under the tgt_prog
      condition.
    - Fix some indentation issues.

Changes in v7:
    - Replace attach_depth with a boolean flag to indicate a program is
      already tracing an fentry/fexit.

Changes in v6:
    - Apply nesting level limitation only to tracing programs, otherwise
      it's possible to apply it in "fentry->extension" case and break it

Changes in v5:
    - Remove follower_cnt and drop unreachable cycle prevention condition
    - Allow only one level of attachment nesting
    - Do not display attach_depth in bpftool, as it doesn't make sense
      anymore

Changes in v3:
    - Fix incorrect decreasing of attach_depth, setting to 0 instead
    - Place bookkeeping later, to not miss a cleanup if needed
    - Display attach_depth in bpftool only if the value is not 0

Changes in v2:
    - Verify tgt_prog is not null
    - Replace boolean followed with number of followers, to handle
      multiple progs attaching/detaching

 include/linux/bpf.h   |  1 +
 kernel/bpf/syscall.c  | 10 +++++++++-
 kernel/bpf/verifier.c | 39 +++++++++++++++++++++++++--------------
 3 files changed, 35 insertions(+), 15 deletions(-)

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index eb447b0a9423..e7393674ab94 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -1414,6 +1414,7 @@ struct bpf_prog_aux {
 	bool dev_bound; /* Program is bound to the netdev. */
 	bool offload_requested; /* Program is bound and offloaded to the netdev. */
 	bool attach_btf_trace; /* true if attaching to BTF-enabled raw tp */
+	bool attach_tracing_prog; /* true if tracing another tracing program */
 	bool func_proto_unreliable;
 	bool sleepable;
 	bool tail_call_reachable;
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 5e43ddd1b83f..bcc5d5ab0870 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -3040,8 +3040,10 @@ static void bpf_tracing_link_release(struct bpf_link *link)
 	bpf_trampoline_put(tr_link->trampoline);
 
 	/* tgt_prog is NULL if target is a kernel function */
-	if (tr_link->tgt_prog)
+	if (tr_link->tgt_prog) {
 		bpf_prog_put(tr_link->tgt_prog);
+		link->prog->aux->attach_tracing_prog = false;
+	}
 }
 
 static void bpf_tracing_link_dealloc(struct bpf_link *link)
@@ -3243,6 +3245,12 @@ static int bpf_tracing_prog_attach(struct bpf_prog *prog,
 		goto out_unlock;
 	}
 
+	/* Bookkeeping for managing the prog attachment chain */
+	if (tgt_prog &&
+	    prog->type == BPF_PROG_TYPE_TRACING &&
+	    tgt_prog->type == BPF_PROG_TYPE_TRACING)
+		prog->aux->attach_tracing_prog = true;
+
 	link->tgt_prog = tgt_prog;
 	link->trampoline = tr;
 
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 8e7b6072e3f4..f8c15ce8fd05 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -20077,6 +20077,7 @@ int bpf_check_attach_target(struct bpf_verifier_log *log,
 			    struct bpf_attach_target_info *tgt_info)
 {
 	bool prog_extension = prog->type == BPF_PROG_TYPE_EXT;
+	bool prog_tracing = prog->type == BPF_PROG_TYPE_TRACING;
 	const char prefix[] = "btf_trace_";
 	int ret = 0, subprog = -1, i;
 	const struct btf_type *t;
@@ -20147,10 +20148,21 @@ int bpf_check_attach_target(struct bpf_verifier_log *log,
 			bpf_log(log, "Can attach to only JITed progs\n");
 			return -EINVAL;
 		}
-		if (tgt_prog->type == prog->type) {
-			/* Cannot fentry/fexit another fentry/fexit program.
-			 * Cannot attach program extension to another extension.
-			 * It's ok to attach fentry/fexit to extension program.
+		if (prog_tracing) {
+			if (aux->attach_tracing_prog) {
+				/*
+				 * Target program is an fentry/fexit which is already attached
+				 * to another tracing program. More levels of nesting
+				 * attachment are not allowed.
+				 */
+				bpf_log(log, "Cannot nest tracing program attach more than once\n");
+				return -EINVAL;
+			}
+		} else if (tgt_prog->type == prog->type) {
+			/*
+			 * To avoid potential call chain cycles, prevent attaching of a
+			 * program extension to another extension. It's ok to attach
+			 * fentry/fexit to extension program.
 			 */
 			bpf_log(log, "Cannot recursively attach\n");
 			return -EINVAL;
@@ -20163,16 +20175,15 @@ int bpf_check_attach_target(struct bpf_verifier_log *log,
 			 * except fentry/fexit. The reason is the following.
 			 * The fentry/fexit programs are used for performance
 			 * analysis, stats and can be attached to any program
-			 * type except themselves. When extension program is
-			 * replacing XDP function it is necessary to allow
-			 * performance analysis of all functions. Both original
-			 * XDP program and its program extension. Hence
-			 * attaching fentry/fexit to BPF_PROG_TYPE_EXT is
-			 * allowed. If extending of fentry/fexit was allowed it
-			 * would be possible to create long call chain
-			 * fentry->extension->fentry->extension beyond
-			 * reasonable stack size. Hence extending fentry is not
-			 * allowed.
+			 * type. When extension program is replacing XDP function
+			 * it is necessary to allow performance analysis of all
+			 * functions. Both original XDP program and its program
+			 * extension. Hence attaching fentry/fexit to
+			 * BPF_PROG_TYPE_EXT is allowed. If extending of
+			 * fentry/fexit was allowed it would be possible to create
+			 * long call chain fentry->extension->fentry->extension
+			 * beyond reasonable stack size. Hence extending fentry
+			 * is not allowed.
 			 */
 			bpf_log(log, "Cannot extend fentry/fexit\n");
 			return -EINVAL;
-- 
2.41.0

[PATCH bpf-next v9 2/4] selftests/bpf: Add test for recursive attachment of tracing progs

[Dmitrii Dolgov]

Verify the fact that only one fentry prog could be attached to another
fentry, building up an attachment chain of limited size. Use existing
bpf_testmod as a start of the chain.

Signed-off-by: Dmitrii Dolgov <9erthalion6@gmail.com>
---
Changes in v8:
    - Cleanup test bpf progs and the content of first/second condition
      in the loop.

Changes in v5:
    - Test only one level of attachment

 .../bpf/prog_tests/recursive_attach.c         | 68 +++++++++++++++++++
 .../selftests/bpf/progs/fentry_recursive.c    | 16 +++++
 .../bpf/progs/fentry_recursive_target.c       | 17 +++++
 3 files changed, 101 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/recursive_attach.c
 create mode 100644 tools/testing/selftests/bpf/progs/fentry_recursive.c
 create mode 100644 tools/testing/selftests/bpf/progs/fentry_recursive_target.c

diff --git a/tools/testing/selftests/bpf/prog_tests/recursive_attach.c b/tools/testing/selftests/bpf/prog_tests/recursive_attach.c
new file mode 100644
index 000000000000..5b38783bcd16
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/recursive_attach.c
@@ -0,0 +1,68 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2023 Red Hat, Inc. */
+#include <test_progs.h>
+#include "fentry_recursive.skel.h"
+#include "fentry_recursive_target.skel.h"
+#include <bpf/btf.h>
+#include "bpf/libbpf_internal.h"
+
+/*
+ * Test following scenarios:
+ * - attach one fentry progs to another one
+ * - more than one nesting levels are not allowed
+ */
+void test_recursive_fentry_attach(void)
+{
+	struct fentry_recursive_target *target_skel = NULL;
+	struct fentry_recursive *tracing_chain[2] = {};
+	struct bpf_program *prog;
+	int prev_fd, err;
+
+	target_skel = fentry_recursive_target__open_and_load();
+	if (!ASSERT_OK_PTR(target_skel, "fentry_recursive_target__open_and_load"))
+		goto close_prog;
+
+	/* Create an attachment chain with two fentry progs */
+	for (int i = 0; i < 2; i++) {
+		tracing_chain[i] = fentry_recursive__open();
+		if (!ASSERT_OK_PTR(tracing_chain[i], "fentry_recursive__open"))
+			goto close_prog;
+
+		/*
+		 * The first prog in the chain is going to be attached to the target
+		 * fentry program, the second one to the previous in the chain.
+		 */
+		prog = tracing_chain[i]->progs.recursive_attach;
+		if (i == 0) {
+			prev_fd = bpf_program__fd(target_skel->progs.test1);
+			err = bpf_program__set_attach_target(prog, prev_fd, "test1");
+		} else {
+			prev_fd = bpf_program__fd(tracing_chain[i-1]->progs.recursive_attach);
+			err = bpf_program__set_attach_target(prog, prev_fd, "recursive_attach");
+		}
+
+		if (!ASSERT_OK(err, "bpf_program__set_attach_target"))
+			goto close_prog;
+
+		err = fentry_recursive__load(tracing_chain[i]);
+		/* The first attach should succeed, the second fail */
+		if (i == 0) {
+			if (!ASSERT_OK(err, "fentry_recursive__load"))
+				goto close_prog;
+
+			err = fentry_recursive__attach(tracing_chain[i]);
+			if (!ASSERT_OK(err, "fentry_recursive__attach"))
+				goto close_prog;
+		} else {
+			if (!ASSERT_ERR(err, "fentry_recursive__load"))
+				goto close_prog;
+		}
+	}
+
+close_prog:
+	fentry_recursive_target__destroy(target_skel);
+	for (int i = 0; i < 2; i++) {
+		if (tracing_chain[i])
+			fentry_recursive__destroy(tracing_chain[i]);
+	}
+}
diff --git a/tools/testing/selftests/bpf/progs/fentry_recursive.c b/tools/testing/selftests/bpf/progs/fentry_recursive.c
new file mode 100644
index 000000000000..b9e4d35ac597
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/fentry_recursive.c
@@ -0,0 +1,16 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2023 Red Hat, Inc. */
+#include <linux/bpf.h>
+#include <bpf/bpf_helpers.h>
+#include <bpf/bpf_tracing.h>
+
+char _license[] SEC("license") = "GPL";
+
+/*
+ * Dummy fentry bpf prog for testing fentry attachment chains
+ */
+SEC("fentry/XXX")
+int BPF_PROG(recursive_attach, int a)
+{
+	return 0;
+}
diff --git a/tools/testing/selftests/bpf/progs/fentry_recursive_target.c b/tools/testing/selftests/bpf/progs/fentry_recursive_target.c
new file mode 100644
index 000000000000..6e0b5c716f8e
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/fentry_recursive_target.c
@@ -0,0 +1,17 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2023 Red Hat, Inc. */
+#include <linux/bpf.h>
+#include <bpf/bpf_helpers.h>
+#include <bpf/bpf_tracing.h>
+
+char _license[] SEC("license") = "GPL";
+
+/*
+ * Dummy fentry bpf prog for testing fentry attachment chains. It's going to be
+ * a start of the chain.
+ */
+SEC("fentry/bpf_testmod_fentry_test1")
+int BPF_PROG(test1, int a)
+{
+	return 0;
+}
-- 
2.41.0

[PATCH bpf-next v9 3/4] bpf: Fix re-attachment branch in bpf_tracing_prog_attach

[Dmitrii Dolgov]

From: Jiri Olsa <olsajiri@gmail.com>

The following case can cause a crash due to missing attach_btf:

1) load rawtp program
2) load fentry program with rawtp as target_fd
3) create tracing link for fentry program with target_fd = 0
4) repeat 3

In the end we have:

- prog->aux->dst_trampoline == NULL
- tgt_prog == NULL (because we did not provide target_fd to link_create)
- prog->aux->attach_btf == NULL (the program was loaded with attach_prog_fd=X)
- the program was loaded for tgt_prog but we have no way to find out which one

    BUG: kernel NULL pointer dereference, address: 0000000000000058
    Call Trace:
     <TASK>
     ? __die+0x20/0x70
     ? page_fault_oops+0x15b/0x430
     ? fixup_exception+0x22/0x330
     ? exc_page_fault+0x6f/0x170
     ? asm_exc_page_fault+0x22/0x30
     ? bpf_tracing_prog_attach+0x279/0x560
     ? btf_obj_id+0x5/0x10
     bpf_tracing_prog_attach+0x439/0x560
     __sys_bpf+0x1cf4/0x2de0
     __x64_sys_bpf+0x1c/0x30
     do_syscall_64+0x41/0xf0
     entry_SYSCALL_64_after_hwframe+0x6e/0x76

Return -EINVAL in this situation.

Signed-off-by: Jiri Olsa <olsajiri@gmail.com>
Signed-off-by: Dmitrii Dolgov <9erthalion6@gmail.com>
---
 kernel/bpf/syscall.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index bcc5d5ab0870..e07c3c1086aa 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -3182,6 +3182,10 @@ static int bpf_tracing_prog_attach(struct bpf_prog *prog,
 	 *
 	 * - if prog->aux->dst_trampoline and tgt_prog is NULL, the program
 	 *   was detached and is going for re-attachment.
+	 *
+	 * - if prog->aux->dst_trampoline is NULL and tgt_prog and prog->aux->attach_btf
+	 *   are NULL, then program was already attached and user did not provide
+	 *   tgt_prog_fd so we have no way to find out or create trampoline
 	 */
 	if (!prog->aux->dst_trampoline && !tgt_prog) {
 		/*
@@ -3195,6 +3199,11 @@ static int bpf_tracing_prog_attach(struct bpf_prog *prog,
 			err = -EINVAL;
 			goto out_unlock;
 		}
+		/* We can allow re-attach only if we have valid attach_btf. */
+		if (!prog->aux->attach_btf) {
+			err = -EINVAL;
+			goto out_unlock;
+		}
 		btf_id = prog->aux->attach_btf_id;
 		key = bpf_trampoline_compute_key(NULL, prog->aux->attach_btf, btf_id);
 	}
-- 
2.41.0

[PATCH bpf-next v9 4/4] selftests/bpf: Test re-attachment fix for bpf_tracing_prog_attach

[Dmitrii Dolgov]

Add a test case to verify the fix for "prog->aux->dst_trampoline and
tgt_prog is NULL" branch in bpf_tracing_prog_attach. The sequence of
events:

1. load rawtp program
2. load fentry program with rawtp as target_fd
3. create tracing link for fentry program with target_fd = 0
4. repeat 3

Signed-off-by: Dmitrii Dolgov <9erthalion6@gmail.com>
---
Changes in v8:
    - Cleanup, remove link opts and if condition around assert for the
      expected error, unneeded parts of the test bpf prog and some
      indendation improvements.

 .../bpf/prog_tests/recursive_attach.c         | 45 +++++++++++++++++++
 .../bpf/progs/fentry_recursive_target.c       | 10 +++++
 2 files changed, 55 insertions(+)

diff --git a/tools/testing/selftests/bpf/prog_tests/recursive_attach.c b/tools/testing/selftests/bpf/prog_tests/recursive_attach.c
index 5b38783bcd16..1063b7924343 100644
--- a/tools/testing/selftests/bpf/prog_tests/recursive_attach.c
+++ b/tools/testing/selftests/bpf/prog_tests/recursive_attach.c
@@ -66,3 +66,48 @@ void test_recursive_fentry_attach(void)
 			fentry_recursive__destroy(tracing_chain[i]);
 	}
 }
+
+/*
+ * Test that a tracing prog reattachment (when we land in
+ * "prog->aux->dst_trampoline and tgt_prog is NULL" branch in
+ * bpf_tracing_prog_attach) does not lead to a crash due to missing attach_btf
+ */
+void test_fentry_attach_btf_presence(void)
+{
+	struct fentry_recursive_target *target_skel = NULL;
+	struct fentry_recursive *tracing_skel = NULL;
+	struct bpf_program *prog;
+	int err, link_fd, tgt_prog_fd;
+
+	target_skel = fentry_recursive_target__open_and_load();
+	if (!ASSERT_OK_PTR(target_skel, "fentry_recursive_target__open_and_load"))
+		goto close_prog;
+
+	tracing_skel = fentry_recursive__open();
+	if (!ASSERT_OK_PTR(tracing_skel, "fentry_recursive__open"))
+		goto close_prog;
+
+	prog = tracing_skel->progs.recursive_attach;
+	tgt_prog_fd = bpf_program__fd(target_skel->progs.fentry_target);
+	err = bpf_program__set_attach_target(prog, tgt_prog_fd, "fentry_target");
+	if (!ASSERT_OK(err, "bpf_program__set_attach_target"))
+		goto close_prog;
+
+	err = fentry_recursive__load(tracing_skel);
+	if (!ASSERT_OK(err, "fentry_recursive__load"))
+		goto close_prog;
+
+	tgt_prog_fd = bpf_program__fd(tracing_skel->progs.recursive_attach);
+	link_fd = bpf_link_create(tgt_prog_fd, 0, BPF_TRACE_FENTRY, NULL);
+	if (!ASSERT_GE(link_fd, 0, "link_fd"))
+		goto close_prog;
+
+	fentry_recursive__detach(tracing_skel);
+
+	err = fentry_recursive__attach(tracing_skel);
+	ASSERT_ERR(err, "fentry_recursive__attach");
+
+close_prog:
+	fentry_recursive_target__destroy(target_skel);
+	fentry_recursive__destroy(tracing_skel);
+}
diff --git a/tools/testing/selftests/bpf/progs/fentry_recursive_target.c b/tools/testing/selftests/bpf/progs/fentry_recursive_target.c
index 6e0b5c716f8e..51af8426da3a 100644
--- a/tools/testing/selftests/bpf/progs/fentry_recursive_target.c
+++ b/tools/testing/selftests/bpf/progs/fentry_recursive_target.c
@@ -15,3 +15,13 @@ int BPF_PROG(test1, int a)
 {
 	return 0;
 }
+
+/*
+ * Dummy bpf prog for testing attach_btf presence when attaching an fentry
+ * program.
+ */
+SEC("raw_tp/sys_enter")
+int BPF_PROG(fentry_target, struct pt_regs *regs, long id)
+{
+	return 0;
+}
-- 
2.41.0

Re: [PATCH bpf-next v9 1/4] bpf: Relax tracing prog recursive attach rules

[Song Liu]

On Fri, Dec 15, 2023 at 12:11 PM Dmitrii Dolgov <9erthalion6@gmail.com> wrote:
[...]
> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index eb447b0a9423..e7393674ab94 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -1414,6 +1414,7 @@ struct bpf_prog_aux {
>         bool dev_bound; /* Program is bound to the netdev. */
>         bool offload_requested; /* Program is bound and offloaded to the netdev. */
>         bool attach_btf_trace; /* true if attaching to BTF-enabled raw tp */
> +       bool attach_tracing_prog; /* true if tracing another tracing program */
>         bool func_proto_unreliable;
>         bool sleepable;
>         bool tail_call_reachable;
> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index 5e43ddd1b83f..bcc5d5ab0870 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -3040,8 +3040,10 @@ static void bpf_tracing_link_release(struct bpf_link *link)
>         bpf_trampoline_put(tr_link->trampoline);
>
>         /* tgt_prog is NULL if target is a kernel function */
> -       if (tr_link->tgt_prog)
> +       if (tr_link->tgt_prog) {
>                 bpf_prog_put(tr_link->tgt_prog);
> +               link->prog->aux->attach_tracing_prog = false;
> +       }
>  }
>
>  static void bpf_tracing_link_dealloc(struct bpf_link *link)
> @@ -3243,6 +3245,12 @@ static int bpf_tracing_prog_attach(struct bpf_prog *prog,
>                 goto out_unlock;
>         }
>
> +       /* Bookkeeping for managing the prog attachment chain */
> +       if (tgt_prog &&
> +           prog->type == BPF_PROG_TYPE_TRACING &&
> +           tgt_prog->type == BPF_PROG_TYPE_TRACING)
> +               prog->aux->attach_tracing_prog = true;
> +
>         link->tgt_prog = tgt_prog;
>         link->trampoline = tr;
>
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 8e7b6072e3f4..f8c15ce8fd05 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -20077,6 +20077,7 @@ int bpf_check_attach_target(struct bpf_verifier_log *log,
>                             struct bpf_attach_target_info *tgt_info)
>  {
>         bool prog_extension = prog->type == BPF_PROG_TYPE_EXT;
> +       bool prog_tracing = prog->type == BPF_PROG_TYPE_TRACING;
>         const char prefix[] = "btf_trace_";
>         int ret = 0, subprog = -1, i;
>         const struct btf_type *t;
> @@ -20147,10 +20148,21 @@ int bpf_check_attach_target(struct bpf_verifier_log *log,
>                         bpf_log(log, "Can attach to only JITed progs\n");
>                         return -EINVAL;
>                 }
> -               if (tgt_prog->type == prog->type) {
> -                       /* Cannot fentry/fexit another fentry/fexit program.
> -                        * Cannot attach program extension to another extension.
> -                        * It's ok to attach fentry/fexit to extension program.
> +               if (prog_tracing) {
> +                       if (aux->attach_tracing_prog) {
> +                               /*
> +                                * Target program is an fentry/fexit which is already attached
> +                                * to another tracing program. More levels of nesting
> +                                * attachment are not allowed.
> +                                */
> +                               bpf_log(log, "Cannot nest tracing program attach more than once\n");
> +                               return -EINVAL;
> +                       }

If we add

+ prog->aux->attach_tracing_prog = true;

here. We don't need the changes in syscall.c, right?

IOW, we set attach_tracing_prog at program load time, not attach time.

Would this work?

Thanks,
Song

> +               } else if (tgt_prog->type == prog->type) {
> +                       /*
> +                        * To avoid potential call chain cycles, prevent attaching of a
> +                        * program extension to another extension. It's ok to attach
> +                        * fentry/fexit to extension program.

Re: [PATCH bpf-next v9 1/4] bpf: Relax tracing prog recursive attach rules

[Jiri Olsa]

On Sat, Dec 16, 2023 at 05:31:21PM -0800, Song Liu wrote:
> On Fri, Dec 15, 2023 at 12:11 PM Dmitrii Dolgov <9erthalion6@gmail.com> wrote:
> [...]
> > diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> > index eb447b0a9423..e7393674ab94 100644
> > --- a/include/linux/bpf.h
> > +++ b/include/linux/bpf.h
> > @@ -1414,6 +1414,7 @@ struct bpf_prog_aux {
> >         bool dev_bound; /* Program is bound to the netdev. */
> >         bool offload_requested; /* Program is bound and offloaded to the netdev. */
> >         bool attach_btf_trace; /* true if attaching to BTF-enabled raw tp */
> > +       bool attach_tracing_prog; /* true if tracing another tracing program */
> >         bool func_proto_unreliable;
> >         bool sleepable;
> >         bool tail_call_reachable;
> > diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> > index 5e43ddd1b83f..bcc5d5ab0870 100644
> > --- a/kernel/bpf/syscall.c
> > +++ b/kernel/bpf/syscall.c
> > @@ -3040,8 +3040,10 @@ static void bpf_tracing_link_release(struct bpf_link *link)
> >         bpf_trampoline_put(tr_link->trampoline);
> >
> >         /* tgt_prog is NULL if target is a kernel function */
> > -       if (tr_link->tgt_prog)
> > +       if (tr_link->tgt_prog) {
> >                 bpf_prog_put(tr_link->tgt_prog);
> > +               link->prog->aux->attach_tracing_prog = false;
> > +       }
> >  }
> >
> >  static void bpf_tracing_link_dealloc(struct bpf_link *link)
> > @@ -3243,6 +3245,12 @@ static int bpf_tracing_prog_attach(struct bpf_prog *prog,
> >                 goto out_unlock;
> >         }
> >
> > +       /* Bookkeeping for managing the prog attachment chain */
> > +       if (tgt_prog &&
> > +           prog->type == BPF_PROG_TYPE_TRACING &&
> > +           tgt_prog->type == BPF_PROG_TYPE_TRACING)
> > +               prog->aux->attach_tracing_prog = true;
> > +
> >         link->tgt_prog = tgt_prog;
> >         link->trampoline = tr;
> >
> > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> > index 8e7b6072e3f4..f8c15ce8fd05 100644
> > --- a/kernel/bpf/verifier.c
> > +++ b/kernel/bpf/verifier.c
> > @@ -20077,6 +20077,7 @@ int bpf_check_attach_target(struct bpf_verifier_log *log,
> >                             struct bpf_attach_target_info *tgt_info)
> >  {
> >         bool prog_extension = prog->type == BPF_PROG_TYPE_EXT;
> > +       bool prog_tracing = prog->type == BPF_PROG_TYPE_TRACING;
> >         const char prefix[] = "btf_trace_";
> >         int ret = 0, subprog = -1, i;
> >         const struct btf_type *t;
> > @@ -20147,10 +20148,21 @@ int bpf_check_attach_target(struct bpf_verifier_log *log,
> >                         bpf_log(log, "Can attach to only JITed progs\n");
> >                         return -EINVAL;
> >                 }
> > -               if (tgt_prog->type == prog->type) {
> > -                       /* Cannot fentry/fexit another fentry/fexit program.
> > -                        * Cannot attach program extension to another extension.
> > -                        * It's ok to attach fentry/fexit to extension program.
> > +               if (prog_tracing) {
> > +                       if (aux->attach_tracing_prog) {
> > +                               /*
> > +                                * Target program is an fentry/fexit which is already attached
> > +                                * to another tracing program. More levels of nesting
> > +                                * attachment are not allowed.
> > +                                */
> > +                               bpf_log(log, "Cannot nest tracing program attach more than once\n");
> > +                               return -EINVAL;
> > +                       }
> 
> If we add
> 
> + prog->aux->attach_tracing_prog = true;
> 
> here. We don't need the changes in syscall.c, right?
> 
> IOW, we set attach_tracing_prog at program load time, not attach time.
> 
> Would this work?

I think it'd work.. I can just think of a case where we'd not allow
to attach fentry program (3) to another fentry (2) even if it's not
attached, but just loaded, like:


load (fentry1 -> kernel function)

load (fentry2 -> fentry1)
  fentry2->attach_tracing_prog = true

load (fentry3 -> fentry2)
  if (fentry2->aux->attach_tracing_prog)
    return -EINVAL


I guess it's corner case that does not make much sense, but still it
feels more natural to me to set it in attach time

jirka

> 
> Thanks,
> Song
> 
> > +               } else if (tgt_prog->type == prog->type) {
> > +                       /*
> > +                        * To avoid potential call chain cycles, prevent attaching of a
> > +                        * program extension to another extension. It's ok to attach
> > +                        * fentry/fexit to extension program.

Re: [PATCH bpf-next v9 1/4] bpf: Relax tracing prog recursive attach rules

[Song Liu]

On Sun, Dec 17, 2023 at 11:22 AM Jiri Olsa <olsajiri@gmail.com> wrote:
>
> On Sat, Dec 16, 2023 at 05:31:21PM -0800, Song Liu wrote:
> > On Fri, Dec 15, 2023 at 12:11 PM Dmitrii Dolgov <9erthalion6@gmail.com> wrote:
> > [...]
> > > diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> > > index eb447b0a9423..e7393674ab94 100644
> > > --- a/include/linux/bpf.h
> > > +++ b/include/linux/bpf.h
> > > @@ -1414,6 +1414,7 @@ struct bpf_prog_aux {
> > >         bool dev_bound; /* Program is bound to the netdev. */
> > >         bool offload_requested; /* Program is bound and offloaded to the netdev. */
> > >         bool attach_btf_trace; /* true if attaching to BTF-enabled raw tp */
> > > +       bool attach_tracing_prog; /* true if tracing another tracing program */
> > >         bool func_proto_unreliable;
> > >         bool sleepable;
> > >         bool tail_call_reachable;
> > > diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> > > index 5e43ddd1b83f..bcc5d5ab0870 100644
> > > --- a/kernel/bpf/syscall.c
> > > +++ b/kernel/bpf/syscall.c
> > > @@ -3040,8 +3040,10 @@ static void bpf_tracing_link_release(struct bpf_link *link)
> > >         bpf_trampoline_put(tr_link->trampoline);
> > >
> > >         /* tgt_prog is NULL if target is a kernel function */
> > > -       if (tr_link->tgt_prog)
> > > +       if (tr_link->tgt_prog) {
> > >                 bpf_prog_put(tr_link->tgt_prog);
> > > +               link->prog->aux->attach_tracing_prog = false;
> > > +       }
> > >  }
> > >
> > >  static void bpf_tracing_link_dealloc(struct bpf_link *link)
> > > @@ -3243,6 +3245,12 @@ static int bpf_tracing_prog_attach(struct bpf_prog *prog,
> > >                 goto out_unlock;
> > >         }
> > >
> > > +       /* Bookkeeping for managing the prog attachment chain */
> > > +       if (tgt_prog &&
> > > +           prog->type == BPF_PROG_TYPE_TRACING &&
> > > +           tgt_prog->type == BPF_PROG_TYPE_TRACING)
> > > +               prog->aux->attach_tracing_prog = true;
> > > +
> > >         link->tgt_prog = tgt_prog;
> > >         link->trampoline = tr;
> > >
> > > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> > > index 8e7b6072e3f4..f8c15ce8fd05 100644
> > > --- a/kernel/bpf/verifier.c
> > > +++ b/kernel/bpf/verifier.c
> > > @@ -20077,6 +20077,7 @@ int bpf_check_attach_target(struct bpf_verifier_log *log,
> > >                             struct bpf_attach_target_info *tgt_info)
> > >  {
> > >         bool prog_extension = prog->type == BPF_PROG_TYPE_EXT;
> > > +       bool prog_tracing = prog->type == BPF_PROG_TYPE_TRACING;
> > >         const char prefix[] = "btf_trace_";
> > >         int ret = 0, subprog = -1, i;
> > >         const struct btf_type *t;
> > > @@ -20147,10 +20148,21 @@ int bpf_check_attach_target(struct bpf_verifier_log *log,
> > >                         bpf_log(log, "Can attach to only JITed progs\n");
> > >                         return -EINVAL;
> > >                 }
> > > -               if (tgt_prog->type == prog->type) {
> > > -                       /* Cannot fentry/fexit another fentry/fexit program.
> > > -                        * Cannot attach program extension to another extension.
> > > -                        * It's ok to attach fentry/fexit to extension program.
> > > +               if (prog_tracing) {
> > > +                       if (aux->attach_tracing_prog) {
> > > +                               /*
> > > +                                * Target program is an fentry/fexit which is already attached
> > > +                                * to another tracing program. More levels of nesting
> > > +                                * attachment are not allowed.
> > > +                                */
> > > +                               bpf_log(log, "Cannot nest tracing program attach more than once\n");
> > > +                               return -EINVAL;
> > > +                       }
> >
> > If we add
> >
> > + prog->aux->attach_tracing_prog = true;
> >
> > here. We don't need the changes in syscall.c, right?
> >
> > IOW, we set attach_tracing_prog at program load time, not attach time.
> >
> > Would this work?
>
> I think it'd work.. I can just think of a case where we'd not allow
> to attach fentry program (3) to another fentry (2) even if it's not
> attached, but just loaded, like:
>
>
> load (fentry1 -> kernel function)
>
> load (fentry2 -> fentry1)
>   fentry2->attach_tracing_prog = true
>
> load (fentry3 -> fentry2)
>   if (fentry2->aux->attach_tracing_prog)
>     return -EINVAL
>
>
> I guess it's corner case that does not make much sense, but still it
> feels more natural to me to set it in attach time

If we set attach_tracing_prog at attach time, the following will
succeed:

  load (fentry1 -> kernel function)
  load (fentry2 -> fentry1)
  load (fentry3 -> fentry2)
  attach (fentry1)
  attach (fentry2)
  attach (fentry3)

We can even make attach chain longer, as long as we load
the chain first. This is really confusing to me. So I think we should
set the flag at load() time.

Does this make sense?

Thanks,
Song

Re: [PATCH bpf-next v9 1/4] bpf: Relax tracing prog recursive attach rules

[Jiri Olsa]

On Sun, Dec 17, 2023 at 04:58:07PM -0800, Song Liu wrote:
> On Sun, Dec 17, 2023 at 11:22 AM Jiri Olsa <olsajiri@gmail.com> wrote:
> >
> > On Sat, Dec 16, 2023 at 05:31:21PM -0800, Song Liu wrote:
> > > On Fri, Dec 15, 2023 at 12:11 PM Dmitrii Dolgov <9erthalion6@gmail.com> wrote:
> > > [...]
> > > > diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> > > > index eb447b0a9423..e7393674ab94 100644
> > > > --- a/include/linux/bpf.h
> > > > +++ b/include/linux/bpf.h
> > > > @@ -1414,6 +1414,7 @@ struct bpf_prog_aux {
> > > >         bool dev_bound; /* Program is bound to the netdev. */
> > > >         bool offload_requested; /* Program is bound and offloaded to the netdev. */
> > > >         bool attach_btf_trace; /* true if attaching to BTF-enabled raw tp */
> > > > +       bool attach_tracing_prog; /* true if tracing another tracing program */
> > > >         bool func_proto_unreliable;
> > > >         bool sleepable;
> > > >         bool tail_call_reachable;
> > > > diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> > > > index 5e43ddd1b83f..bcc5d5ab0870 100644
> > > > --- a/kernel/bpf/syscall.c
> > > > +++ b/kernel/bpf/syscall.c
> > > > @@ -3040,8 +3040,10 @@ static void bpf_tracing_link_release(struct bpf_link *link)
> > > >         bpf_trampoline_put(tr_link->trampoline);
> > > >
> > > >         /* tgt_prog is NULL if target is a kernel function */
> > > > -       if (tr_link->tgt_prog)
> > > > +       if (tr_link->tgt_prog) {
> > > >                 bpf_prog_put(tr_link->tgt_prog);
> > > > +               link->prog->aux->attach_tracing_prog = false;
> > > > +       }
> > > >  }
> > > >
> > > >  static void bpf_tracing_link_dealloc(struct bpf_link *link)
> > > > @@ -3243,6 +3245,12 @@ static int bpf_tracing_prog_attach(struct bpf_prog *prog,
> > > >                 goto out_unlock;
> > > >         }
> > > >
> > > > +       /* Bookkeeping for managing the prog attachment chain */
> > > > +       if (tgt_prog &&
> > > > +           prog->type == BPF_PROG_TYPE_TRACING &&
> > > > +           tgt_prog->type == BPF_PROG_TYPE_TRACING)
> > > > +               prog->aux->attach_tracing_prog = true;
> > > > +
> > > >         link->tgt_prog = tgt_prog;
> > > >         link->trampoline = tr;
> > > >
> > > > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> > > > index 8e7b6072e3f4..f8c15ce8fd05 100644
> > > > --- a/kernel/bpf/verifier.c
> > > > +++ b/kernel/bpf/verifier.c
> > > > @@ -20077,6 +20077,7 @@ int bpf_check_attach_target(struct bpf_verifier_log *log,
> > > >                             struct bpf_attach_target_info *tgt_info)
> > > >  {
> > > >         bool prog_extension = prog->type == BPF_PROG_TYPE_EXT;
> > > > +       bool prog_tracing = prog->type == BPF_PROG_TYPE_TRACING;
> > > >         const char prefix[] = "btf_trace_";
> > > >         int ret = 0, subprog = -1, i;
> > > >         const struct btf_type *t;
> > > > @@ -20147,10 +20148,21 @@ int bpf_check_attach_target(struct bpf_verifier_log *log,
> > > >                         bpf_log(log, "Can attach to only JITed progs\n");
> > > >                         return -EINVAL;
> > > >                 }
> > > > -               if (tgt_prog->type == prog->type) {
> > > > -                       /* Cannot fentry/fexit another fentry/fexit program.
> > > > -                        * Cannot attach program extension to another extension.
> > > > -                        * It's ok to attach fentry/fexit to extension program.
> > > > +               if (prog_tracing) {
> > > > +                       if (aux->attach_tracing_prog) {
> > > > +                               /*
> > > > +                                * Target program is an fentry/fexit which is already attached
> > > > +                                * to another tracing program. More levels of nesting
> > > > +                                * attachment are not allowed.
> > > > +                                */
> > > > +                               bpf_log(log, "Cannot nest tracing program attach more than once\n");
> > > > +                               return -EINVAL;
> > > > +                       }
> > >
> > > If we add
> > >
> > > + prog->aux->attach_tracing_prog = true;
> > >
> > > here. We don't need the changes in syscall.c, right?
> > >
> > > IOW, we set attach_tracing_prog at program load time, not attach time.
> > >
> > > Would this work?
> >
> > I think it'd work.. I can just think of a case where we'd not allow
> > to attach fentry program (3) to another fentry (2) even if it's not
> > attached, but just loaded, like:
> >
> >
> > load (fentry1 -> kernel function)
> >
> > load (fentry2 -> fentry1)
> >   fentry2->attach_tracing_prog = true
> >
> > load (fentry3 -> fentry2)
> >   if (fentry2->aux->attach_tracing_prog)
> >     return -EINVAL
> >
> >
> > I guess it's corner case that does not make much sense, but still it
> > feels more natural to me to set it in attach time
> 
> If we set attach_tracing_prog at attach time, the following will
> succeed:
> 
>   load (fentry1 -> kernel function)
>   load (fentry2 -> fentry1)
>   load (fentry3 -> fentry2)
>   attach (fentry1)
>   attach (fentry2)
>   attach (fentry3)
> 
> We can even make attach chain longer, as long as we load
> the chain first. This is really confusing to me. So I think we should
> set the flag at load() time.
> 
> Does this make sense?

yes, I did not consider this option.. makes sense

thanks,
jirka

Re: [PATCH bpf-next v9 1/4] bpf: Relax tracing prog recursive attach rules

[Dmitry Dolgov]

> On Mon, Dec 18, 2023 at 08:47:14AM +0100, Jiri Olsa wrote:
> > > > If we add
> > > >
> > > > + prog->aux->attach_tracing_prog = true;
> > > >
> > > > here. We don't need the changes in syscall.c, right?
> > > >
> > > > IOW, we set attach_tracing_prog at program load time, not attach time.
> > > >
> > > > Would this work?
> > >
> > > I think it'd work.. I can just think of a case where we'd not allow
> > > to attach fentry program (3) to another fentry (2) even if it's not
> > > attached, but just loaded, like:
> > >
> > >
> > > load (fentry1 -> kernel function)
> > >
> > > load (fentry2 -> fentry1)
> > >   fentry2->attach_tracing_prog = true
> > >
> > > load (fentry3 -> fentry2)
> > >   if (fentry2->aux->attach_tracing_prog)
> > >     return -EINVAL
> > >
> > >
> > > I guess it's corner case that does not make much sense, but still it
> > > feels more natural to me to set it in attach time
> >
> > If we set attach_tracing_prog at attach time, the following will
> > succeed:
> >
> >   load (fentry1 -> kernel function)
> >   load (fentry2 -> fentry1)
> >   load (fentry3 -> fentry2)
> >   attach (fentry1)
> >   attach (fentry2)
> >   attach (fentry3)
> >
> > We can even make attach chain longer, as long as we load
> > the chain first. This is really confusing to me. So I think we should
> > set the flag at load() time.
> >
> > Does this make sense?
>
> yes, I did not consider this option.. makes sense

Thanks for pointing out this case folks, haven't thought about this
either.

Just for me to clarify explicitly, the reason why the case (loading a
chain and only then attaching every program) would work is that there is
no additional bpf_check_attach_target in bpf_tracing_prog_attach when
the trampoline is getting reused. I've tried to change this a little, it
seems to possible to prevent such situation, and still keep
attach_tracing_prog flag at attach time if there will be one more
bpf_check_attach_target.

At the same time setting attach_tracing_prog flag at load time would be
probably less invasive, making it more preferable I guess. Will post the
new patch series with this change soon.

Re: [PATCH bpf-next v9 1/4] bpf: Relax tracing prog recursive attach rules

[Dmitry Dolgov]

> On Mon, Dec 18, 2023 at 09:10:19PM +0100, Dmitry Dolgov wrote:
> > > > I guess it's corner case that does not make much sense, but still it
> > > > feels more natural to me to set it in attach time
> > >
> > > If we set attach_tracing_prog at attach time, the following will
> > > succeed:
> > >
> > >   load (fentry1 -> kernel function)
> > >   load (fentry2 -> fentry1)
> > >   load (fentry3 -> fentry2)
> > >   attach (fentry1)
> > >   attach (fentry2)
> > >   attach (fentry3)
> > >
> > > We can even make attach chain longer, as long as we load
> > > the chain first. This is really confusing to me. So I think we should
> > > set the flag at load() time.
> > >
> > > Does this make sense?
> >
> > yes, I did not consider this option.. makes sense
>
> Thanks for pointing out this case folks, haven't thought about this
> either.
>
> Just for me to clarify explicitly, the reason why the case (loading a
> chain and only then attaching every program) would work is that there is
> no additional bpf_check_attach_target in bpf_tracing_prog_attach when
> the trampoline is getting reused. I've tried to change this a little, it
> seems to possible to prevent such situation, and still keep
> attach_tracing_prog flag at attach time if there will be one more
> bpf_check_attach_target.
>
> At the same time setting attach_tracing_prog flag at load time would be
> probably less invasive, making it more preferable I guess. Will post the
> new patch series with this change soon.

Thinking about this more, it seems setting attach_tracing_prog flag at
load time should be done not as a replacement (what I've assumed
originally), but in addition to setting it at attach time. Otherwise,
reattaching the same prog will lead to an inconsistency.

Re: [PATCH bpf-next v9 1/4] bpf: Relax tracing prog recursive attach rules

[Dmitry Dolgov]

> On Wed, Dec 20, 2023 at 08:55:43AM +0100, Dmitry Dolgov wrote:
> > On Mon, Dec 18, 2023 at 09:10:19PM +0100, Dmitry Dolgov wrote:
> > > > > I guess it's corner case that does not make much sense, but still it
> > > > > feels more natural to me to set it in attach time
> > > >
> > > > If we set attach_tracing_prog at attach time, the following will
> > > > succeed:
> > > >
> > > >   load (fentry1 -> kernel function)
> > > >   load (fentry2 -> fentry1)
> > > >   load (fentry3 -> fentry2)
> > > >   attach (fentry1)
> > > >   attach (fentry2)
> > > >   attach (fentry3)
> > > >
> > > > We can even make attach chain longer, as long as we load
> > > > the chain first. This is really confusing to me. So I think we should
> > > > set the flag at load() time.
> > > >
> > > > Does this make sense?
> > >
> > > yes, I did not consider this option.. makes sense
> >
> > Thanks for pointing out this case folks, haven't thought about this
> > either.
> >
> > Just for me to clarify explicitly, the reason why the case (loading a
> > chain and only then attaching every program) would work is that there is
> > no additional bpf_check_attach_target in bpf_tracing_prog_attach when
> > the trampoline is getting reused. I've tried to change this a little, it
> > seems to possible to prevent such situation, and still keep
> > attach_tracing_prog flag at attach time if there will be one more
> > bpf_check_attach_target.
> >
> > At the same time setting attach_tracing_prog flag at load time would be
> > probably less invasive, making it more preferable I guess. Will post the
> > new patch series with this change soon.
>
> Thinking about this more, it seems setting attach_tracing_prog flag at
> load time should be done not as a replacement (what I've assumed
> originally), but in addition to setting it at attach time. Otherwise,
> reattaching the same prog will lead to an inconsistency.

On the second thought, at the moment it's not allowed to change an
attachment target for a tracing prog. Which means the attach_tracing_prog
flag could be set for the whole lifetime of a tracing prog, from load
until release -- this way there is even no need to change
bpf_tracing_link_release, and the fist patch will become smaller.