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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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





[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux