Re: [PATCH bpf] bpf: Fix tr dereferencing

[Leon Hwang <hffilwlqm@gmail.com>]

On 2023/9/16 01:38, Alexei Starovoitov wrote:
> 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.

OK, I'll do it in v2 patch.

Thanks,
Leon