* [PATCH bpf] bpf: Fix tr dereferencing
@ 2023-09-14 14:51 Leon Hwang
2023-09-14 21:49 ` Jiri Olsa
2023-09-15 2:13 ` Hengqi Chen
0 siblings, 2 replies; 7+ messages in thread
From: Leon Hwang @ 2023-09-14 14:51 UTC (permalink / raw)
To: bpf
Cc: ast, andrii, daniel, toke, sdf, lkp, dan.carpenter,
maciej.fijalkowski, hengqi.chen, hffilwlqm, kernel-patches-bot
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;
}
} 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
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH bpf] bpf: Fix tr dereferencing
2023-09-14 14:51 [PATCH bpf] bpf: Fix tr dereferencing Leon Hwang
@ 2023-09-14 21:49 ` Jiri Olsa
2023-09-15 2:13 ` Hengqi Chen
1 sibling, 0 replies; 7+ messages in thread
From: Jiri Olsa @ 2023-09-14 21:49 UTC (permalink / raw)
To: Leon Hwang
Cc: bpf, ast, andrii, daniel, toke, sdf, lkp, dan.carpenter,
maciej.fijalkowski, hengqi.chen, kernel-patches-bot
On Thu, Sep 14, 2023 at 10:51:26PM +0800, Leon Hwang 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>
it does not apply cleanly on bpf/master for me, but nice catch
Acked-by: Jiri Olsa <jolsa@kernel.org>
jirka
> ---
> 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;
> }
> } 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
>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH bpf] bpf: Fix tr dereferencing
2023-09-14 14:51 [PATCH bpf] bpf: Fix tr dereferencing Leon Hwang
2023-09-14 21:49 ` Jiri Olsa
@ 2023-09-15 2:13 ` Hengqi Chen
2023-09-15 2:18 ` Leon Hwang
1 sibling, 1 reply; 7+ messages in thread
From: Hengqi Chen @ 2023-09-15 2:13 UTC (permalink / raw)
To: Leon Hwang
Cc: bpf, ast, andrii, daniel, toke, sdf, lkp, dan.carpenter,
maciej.fijalkowski, kernel-patches-bot
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.
> }
> } 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
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH bpf] bpf: Fix tr dereferencing
2023-09-15 2:13 ` Hengqi Chen
@ 2023-09-15 2:18 ` Leon Hwang
2023-09-15 2:54 ` Hengqi Chen
0 siblings, 1 reply; 7+ messages in thread
From: Leon Hwang @ 2023-09-15 2:18 UTC (permalink / raw)
To: Hengqi Chen
Cc: bpf, ast, andrii, daniel, toke, sdf, lkp, dan.carpenter,
maciej.fijalkowski, kernel-patches-bot
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.
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
>>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH bpf] bpf: Fix tr dereferencing
2023-09-15 2:18 ` Leon Hwang
@ 2023-09-15 2:54 ` Hengqi Chen
2023-09-15 17:38 ` Alexei Starovoitov
0 siblings, 1 reply; 7+ messages in thread
From: Hengqi Chen @ 2023-09-15 2:54 UTC (permalink / raw)
To: Leon Hwang
Cc: bpf, ast, andrii, daniel, toke, sdf, lkp, dan.carpenter,
maciej.fijalkowski, kernel-patches-bot
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
> >>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH bpf] bpf: Fix tr dereferencing
2023-09-15 2:54 ` Hengqi Chen
@ 2023-09-15 17:38 ` Alexei Starovoitov
2023-09-17 14:29 ` Leon Hwang
0 siblings, 1 reply; 7+ messages in thread
From: Alexei Starovoitov @ 2023-09-15 17:38 UTC (permalink / raw)
To: Hengqi Chen
Cc: Leon Hwang, bpf, Alexei Starovoitov, Andrii Nakryiko,
Daniel Borkmann, Toke Høiland-Jørgensen,
Stanislav Fomichev, kbuild test robot, Dan Carpenter,
Fijalkowski, Maciej, kernel-patches-bot
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.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH bpf] bpf: Fix tr dereferencing
2023-09-15 17:38 ` Alexei Starovoitov
@ 2023-09-17 14:29 ` Leon Hwang
0 siblings, 0 replies; 7+ messages in thread
From: Leon Hwang @ 2023-09-17 14:29 UTC (permalink / raw)
To: Alexei Starovoitov, Hengqi Chen
Cc: bpf, Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
Toke Høiland-Jørgensen, Stanislav Fomichev,
kbuild test robot, Dan Carpenter, Fijalkowski, Maciej,
kernel-patches-bot
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
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2023-09-17 14:29 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-09-14 14:51 [PATCH bpf] bpf: Fix tr dereferencing Leon Hwang
2023-09-14 21:49 ` Jiri Olsa
2023-09-15 2:13 ` Hengqi Chen
2023-09-15 2:18 ` Leon Hwang
2023-09-15 2:54 ` Hengqi Chen
2023-09-15 17:38 ` Alexei Starovoitov
2023-09-17 14:29 ` Leon Hwang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox