Re: [PATCH bpf] bpf: Fix tr dereferencing

Alexei Starovoitov
2023-09-15T17:39:38Z

On Thu, Sep 14, 2023 at 7:54 PM Hengqi Chen <hengqi.chen@gmail.com> wrote:
>
> On Fri, Sep 15, 2023 at 10:18 AM Leon Hwang <hffilwlqm@gmail.com> wrote:
> >
> >
> >
> > On 15/9/23 10:13, Hengqi Chen wrote:
> > > On Thu, Sep 14, 2023 at 10:51 PM Leon Hwang <hffilwlqm@gmail.com> wrote:
> > >>
> > >> Fix 'tr' dereferencing bug when CONFIG_BPF_JIT is turned off.
> > >>
> > >> Like 'bpf_trampoline_get_progs()', return 'ERR_PTR()' and then check by
> > >> 'IS_ERR()'. As a result, when CONFIG_BPF_JIT is turned off, it's able to
> > >> handle the case that 'bpf_trampoline_get()' returns
> > >> 'ERR_PTR(-EOPNOTSUPP)'.
> > >>
> > >> Fixes: 4a1e7c0c63e0 ("bpf: Support attaching freplace programs to multiple attach points")
> > >> Fixes: f7b12b6fea00 ("bpf: verifier: refactor check_attach_btf_id()")
> > >> Fixes: 69fd337a975c ("bpf: per-cgroup lsm flavor")
> > >> Reported-by: kernel test robot <lkp@intel.com>
> > >> Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
> > >> Closes: https://lore.kernel.org/r/202309131936.5Nc8eUD0-lkp@intel.com/
> > >> Signed-off-by: Leon Hwang <hffilwlqm@gmail.com>
> > >> ---
> > >> kernel/bpf/syscall.c | 4 ++--
> > >> kernel/bpf/trampoline.c | 6 +++---
> > >> kernel/bpf/verifier.c | 4 ++--
> > >> 3 files changed, 7 insertions(+), 7 deletions(-)
> > >>
> > >> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> > >> index 6a692f3bea150..5748d01c99854 100644
> > >> --- a/kernel/bpf/syscall.c
> > >> +++ b/kernel/bpf/syscall.c
> > >> @@ -3211,8 +3211,8 @@ static int bpf_tracing_prog_attach(struct bpf_prog *prog,
> > >> }
> > >>
> > >> tr = bpf_trampoline_get(key, &tgt_info);
> > >> - if (!tr) {
> > >> - err = -ENOMEM;
> > >> + if (IS_ERR(tr)) {
> > >> + err = PTR_ERR(tr);
> > >> goto out_unlock;
> > >
> > > IS_ERR does not check the null case, so this should be IS_ERR_OR_NULL instead.
> >
> > Actually, bpf_trampoline_get() would not return NULL. It returns ERR_PTR(-ENOMEM)
> > or a valid ptr.
> >
>
> OK, I missed the change in bpf_trampoline_get(). Anyway,
>
> Reviewed-by: Hengqi Chen <hengqi.chen@gmail.com>

That's too much churn to address !JIT config.
Just make it return NULL in that case,
instead of hacking things all over the place.