Re: [PATCH bpf] bpf: Fix tr dereferencing

Hengqi Chen
2023-09-15T02:54:58Z

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>

> Thanks,
> Leon
>
> >
> >> }
> >> } else {
> >> diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c
> >> index e97aeda3a86b5..1952614778433 100644
> >> --- a/kernel/bpf/trampoline.c
> >> +++ b/kernel/bpf/trampoline.c
> >> @@ -697,8 +697,8 @@ int bpf_trampoline_link_cgroup_shim(struct bpf_prog *prog,
> >>
> >> bpf_lsm_find_cgroup_shim(prog, &bpf_func);
> >> tr = bpf_trampoline_get(key, &tgt_info);
> >> - if (!tr)
> >> - return -ENOMEM;
> >> + if (IS_ERR(tr))
> >> + return PTR_ERR(tr);
> >>
> >> mutex_lock(&tr->mutex);
> >>
> >> @@ -775,7 +775,7 @@ struct bpf_trampoline *bpf_trampoline_get(u64 key,
> >>
> >> tr = bpf_trampoline_lookup(key);
> >> if (!tr)
> >> - return NULL;
> >> + return ERR_PTR(-ENOMEM);
> >>
> >> mutex_lock(&tr->mutex);
> >> if (tr->func.addr)
> >> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> >> index 18e673c0ac159..054063ead0e54 100644
> >> --- a/kernel/bpf/verifier.c
> >> +++ b/kernel/bpf/verifier.c
> >> @@ -19771,8 +19771,8 @@ static int check_attach_btf_id(struct bpf_verifier_env *env)
> >>
> >> key = bpf_trampoline_compute_key(tgt_prog, prog->aux->attach_btf, btf_id);
> >> tr = bpf_trampoline_get(key, &tgt_info);
> >> - if (!tr)
> >> - return -ENOMEM;
> >> + if (IS_ERR(tr))
> >> + return PTR_ERR(tr);
> >>
> >> if (tgt_prog && tgt_prog->aux->tail_call_reachable)
> >> tr->flags = BPF_TRAMP_F_TAIL_CALL_CTX;
> >>
> >> base-commit: cbb1dbcd99b0ae74c45c4c83c6d213c12c31785c
> >> --
> >> 2.41.0
> >>