On Thu, Nov 30, 2023 at 2:08 AM Dmitry Dolgov <9erthalion6@xxxxxxxxx> wrote: > > > On Wed, Nov 29, 2023 at 03:58:02PM -0800, Song Liu wrote: > > We discussed this in earlier version: > > > > " > > > 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? > > > > Nope, with the proposed changes it still wouldn't be possible to > > reattach B to A (if we're talking about tracing progs of course), > > because this time B is an attachment target on its own. > > " > > > > I think this can be problematic for some users. Basically, doing > > profiling on prog B can cause it to not work (cannot re-attach). > > Sorry, I was probably not clear enough about this first time. Let me > elaborate: > > * The patch affects only tracing programs (only they can reach the > corresponding verifier change), so I assume in your example at least B > and A are fentry/fexit. > > * The patch is less restrictive than the current kernel implementation. > Currently, no attach of a tracing program to another tracing program is > possible, thus IIUC the case you describe (A, B: tracing, C -> B -> A, > then re-attach B -> A) is not possible without the patch (the first B > -> A is going to return a verifier error). Yes, I was aware this is less restrictive than current rules, and I think this can be very useful. > * I've also tried to reproduce this use case with the patch, and noticed > that link_detach is not supported for tracing progs. Which means the > re-attach part in (C -> B -> A) has to be done via unloading of prog B > and C, then reattaching them one-by-one back. This is another > limitation why the case above doesn't seem to be possible (attaching > one-by-one back would of course work without any issues even with the > patch). I think there is an issue without re-attach: 1. Load program A, B, C; 2. Attach C to B; 3. Attach B to A will fail. > Does it all make sense to you, or am I missing something about the > problem you describe? > > > Given it is not possible to create a call circle, shall we remove > > this issue? > > I was originally thinking about this when preparing the patch, even > independently of the question above, simply remove verifier limitation > for an impossible situation sounds interesting. I see the following > pros/cons: > > * Just remove the verifier limitation on recursive attachment is of > course easier. > > * At the same time it makes the implementation less "defensive" against > future changes. > > * Tracking attachment depth & followers might be useful in some other > contexts. > > All in all I've decided that more elaborated approach is slightly > better. But if everyone in the community agrees that less > "defensiveness" is not an issue and verifier could be simply made less > restrictive, I'm fine with that. What do you think? I think the follower_cnt check is not necessary, and may cause confusions. For tracing programs, we are very specific on "which function(s) are we tracing". So I don't think circular attachment can be a real issue. Do we have potential use cases that make the circular attach possible? Thanks, Song
