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

[Jiri Olsa <olsajiri@gmail.com>]

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