On 11/24/23 4:16 PM, Dmitry Dolgov wrote:
On Thu, Nov 23, 2023 at 11:24:34PM -0800, Song Liu wrote:
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?
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.
IIUC, the 'prog B attached to prog A, and prog C attached to prog B'
not really possible.
After prog B attached to prog A, we have
prog B follower_cnt = 1
prog A attach_depth = 1
Then prog C wants to attach to prog B,
since we have prog B follower_cnt = 1, then attaching will fail.
If we do have A <- B <- C chain by
first prog C attached to prog B, and then prog B attached to A
now we have
prog B/C follower_cnt = 1
prog A/B attach_depth = 1
after detaching B from A, we have
prog B follower_cnt = 0
prog A attach_depth = 0
In this particular case, prog B attaching to prog A should succeed
since prog B follower_cnt = 0.
Did I miss anything?
In the commit message, 'falcosecurity libs project' is mentioned as a use
case for chained fentry/fexit bpf programs. I think you should expand the
use case in more details. It is possible with use case description, people
might find better/alternative solutions for your use case.
Also, if you can have a test case to exercise your commit logic,
it will be even better.
+ 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...
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?
Right, it's going to be always 0 for the just loaded program -- but here
in verifier we check attach_depth of the target program, which is
calculated at some point before. Or were you asking about something else?
