On Thu, Nov 30, 2023 at 07:57:52PM +0100, Dmitry Dolgov wrote:
> > On Thu, Nov 30, 2023 at 03:30:38PM +0100, Jiri Olsa wrote:
> > > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> > > index 8e7b6072e3f4..31ffcffb7198 100644
> > > --- a/kernel/bpf/verifier.c
> > > +++ b/kernel/bpf/verifier.c
> > > @@ -20109,6 +20109,12 @@ int bpf_check_attach_target(struct bpf_verifier_log *log,
> > > if (tgt_prog) {
> > > 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;
> > > + }
> >
> > IIUC the use case you have is to be able to attach fentry program on
> > another fentry program.. do you have use case that needs more than
> > that?
> >
> > could we allow just single nesting? that might perhaps endup in easier
> > code while still allowing your use case?
>
> Right, there is no use case beyond attaching an fentry to another one,
> so having just a single nesting should be fine.
>
> > > + /*
> > > + * To avoid potential call chain cycles, prevent attaching programs
> > > + * of the same type. The only exception is standalone fentry/fexit
> > > + * programs that themselves are not attachment targets.
> > > + * That means:
> > > + * - Cannot attach followed fentry/fexit to another
> > > + * fentry/fexit program.
> > > + * - Cannot attach program extension to another extension.
> > > * It's ok to attach fentry/fexit to extension program.
> >
> > next condition below denies extension on fentry/fexit and the reason
> > is the possibility:
> > "... to create long call chain * fentry->extension->fentry->extension
> > beyond reasonable stack size ..."
> >
> > that might be problem also here with 32 allowed nesting
>
> Reducing nesting to only one level probably will lift this question, but
> for posterity, what kind of problem similar to "fentry->extension->fentry->..."
> do you have in mind?
I meant that allowing that amount of nesting will make it easier to get
to a point where we use all the available stack size
jirka
