Re: [PATCH bpf-next v3 12/17] bpf: Disallow fentry/fexit/freplace for exception callbacks

Alexei Starovoitov
2023-09-18T01:56:23Z

On Sat, Sep 16, 2023 at 12:35 PM Kumar Kartikeya Dwivedi
<memxor@gmail.com> wrote:
>
> On Sat, 16 Sept 2023 at 19:30, Kumar Kartikeya Dwivedi <memxor@gmail.com> wrote:
> >
> > On Sat, 16 Sept 2023 at 18:44, Alexei Starovoitov
> > <alexei.starovoitov@gmail.com> wrote:
> > >
> > > On Thu, Sep 14, 2023 at 5:13 AM Kumar Kartikeya Dwivedi
> > > <memxor@gmail.com> wrote:
> > > > > > }
> > > > > > + if (aux->func && aux->func[subprog]->aux->exception_cb) {
> > > > > > + bpf_log(log,
> > > > > > + "Extension programs cannot replace exception callback\n");
> > > > > > + return -EINVAL;
> > > > > > + }
> > > > >
> > > > > This check is redundant because you already did this check above if (prog_extension branch)
> > > > > Remove this as it will never be reached.
> > > > >
> > > >
> > > > Good catch, will fix it in v4.
> > >
> > > No worries. I fixed this duplicate check while applying.
> > > Everything else can be addressed in the follow ups.
> > >
> > > This spam is a bit annoying:
> > > $ ./test_progs -t exceptions
> > > func#0 @0
> > > FENTRY/FEXIT programs cannot attach to exception callback
> > > processed 0 insns (limit 1000000) max_states_per_insn 0 total_states 0
> > > peak_states 0 mark_read 0
> > >
> > > func#0 @0
> > > FENTRY/FEXIT programs cannot attach to exception callback
> > > processed 0 insns (limit 1000000) max_states_per_insn 0 total_states 0
> > > peak_states 0 mark_read 0
> >
> > Thanks for fixing it while applying. I will send a follow up to
> > silence these logs today.
>
> For some reason, I don't seem to see these when just running
> ./test_progs -t exceptions.

That's odd. We need to debug the difference.
I definitely see them and I don't think my setup has anything special.
Would be good for others to confirm.

> I am not sure what I'm doing differently when running the selftests.
> A bit weird, but anyway, just guessing the cause, do you see them when
> you apply this?

Yep. The patch fixes it for me. Pls submit officially.