> On Thu, Dec 21, 2023 at 07:02:02PM +0100, Jiri Olsa wrote:
> > + /*
> > + * Bookkeeping for managing the program attachment chain.
> > + *
> > + * It might be tempting to set attach_tracing_prog flag at the attachment
> > + * time, but this will not prevent from loading bunch of tracing prog
> > + * first, then attach them one to another.
>
> hi,
> sorry for delayed response.. this part gets trickier with every change :-)
Yeah, I'm impressed how many scenarios this one-liner can affect.
> > + *
> > + * The flag attach_tracing_prog is set for the whole program lifecycle, and
> > + * doesn't have to be cleared in bpf_tracing_link_release, since tracing
> > + * programs cannot change attachment target.
>
> I'm not sure that's the case.. AFAICS the bpf_tracing_prog_attach can
> be called on already loaded program with different target program it
> was loaded for, like:
>
> load fentry1 -> bpf_test_fentry1
>
> load fentry2 -> fentry1
> fentry2->attach_tracing_prog = true
>
> load ext1 -> prog
>
> attach fentry2 -> ext1
>
> in which case we drop the tgt_prog from loading time
> and attach fentry2 to ext1
>
> but I think we could just fix with resseting the attach_tracing_prog
> in bpf_tracing_prog_attach when the tgt_prog switch happens
>
> it'd be great to have test for that.. also to find out it's real case,
> I'm not sure I haven't overlooked anything
Before preparing this patch version I was confident it's possible, but
turned out bpf_tracing_prog_attach has this condition:
if (tgt_prog_fd) {
/* For now we only allow new targets for BPF_PROG_TYPE_EXT */
if (prog->type != BPF_PROG_TYPE_EXT) {
err = -EINVAL;
goto out_put_prog;
}
Here is where all such cases I've tried are failing. Just tried what
you've described with an ext prog (reattaching fentry2 via
bpf_link_create with target_fd and link opts containing btf_id) -- the
same result, as well as with trying to change the fentry2 to some
fentry3. Does it make sense to you, or do I miss anything?
As as side note, I find it's generally a good idea to reset
attach_tracing_prog in bpf_tracing_prog_attach when the tgt_prog switch
happens. It has to do both setting it on and off, if the new target is a
tracing/not tracing prog. The flag still will be kept during the whole
lifetime, unless switched in bpf_tracing_prog_attach -- meaning no
changes in bpf_tracing_link_release. If changing the attachment target
would be possible, that would be the way to go.
