On Wed, Nov 22, 2023 at 11:22 AM Dmitrii Dolgov <9erthalion6@xxxxxxxxx> wrote:
>
> Currently, it's not allowed to attach an fentry/fexit prog to another
> one of the same type. 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. Relax "no same type" requirement to "no progs that are
> already an attach target themselves" for the tracing type. In this way
> only a standalone tracing program (without any other progs attached to
> it) could be attached to another one, and no cycle could be formed. To
If prog B attached to prog A, and prog C attached to prog B, then we
detach B. At this point, can we re-attach B to A?
> implement, add a new field into bpf_prog_aux to track the fact of
> attachment in the target prog.
>
[...]
> static void bpf_tracing_link_dealloc(struct bpf_link *link)
> @@ -3235,6 +3238,12 @@ static int bpf_tracing_prog_attach(struct bpf_prog *prog,
> if (err)
> goto out_unlock;
>
> + if (tgt_prog) {
> + /* Bookkeeping for managing the prog attachment chain. */
> + tgt_prog->aux->follower_cnt++;
> + prog->aux->attach_depth = tgt_prog->aux->attach_depth + 1;
> + }
> +
attach_depth is calculated at attach time, so...
> err = bpf_trampoline_link_prog(&link->link, tr);
> if (err) {
> bpf_link_cleanup(&link_primer);
> @@ -4509,6 +4518,7 @@ static int bpf_prog_get_info_by_fd(struct file *file,
> if (prog->aux->btf)
> info.btf_id = btf_obj_id(prog->aux->btf);
> info.attach_btf_id = prog->aux->attach_btf_id;
> + info.attach_depth = prog->aux->attach_depth;
> if (attach_btf)
> info.attach_btf_obj_id = btf_obj_id(attach_btf);
>
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 9ae6eae13471..de058a83d769 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -20329,6 +20329,12 @@ int bpf_check_attach_target(struct bpf_verifier_log *log,
> if (tgt_prog) {
> struct bpf_prog_aux *aux = tgt_prog->aux;
>
> + if (aux->attach_depth >= 32) {
> + bpf_log(log, "Target program attach depth is %d. Too large\n",
> + aux->attach_depth);
> + return -EINVAL;
> + }
> +
(continue from above) attach_depth is always 0 at program load time, no?
Thanks,
Song
> if (bpf_prog_is_dev_bound(prog->aux) &&
> !bpf_prog_dev_bound_match(prog, tgt_prog)) {
> bpf_log(log, "Target program bound device mismatch");
> @@ -20367,9 +20373,16 @@ int bpf_check_attach_target(struct bpf_verifier_log *log,
> bpf_log(log, "Can attach to only JITed progs\n");
> return -EINVAL;
> }
[...]
