> 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.
