* Re: [PATCH bpf-next] bpf: Fix a verifier message for alloc size helper arg
2021-01-12 12:39 [PATCH bpf-next] bpf: Fix a verifier message for alloc size helper arg Brendan Jackman
@ 2021-01-12 13:14 ` KP Singh
2021-01-12 14:55 ` Brendan Jackman
2021-01-12 16:42 ` Yonghong Song
` (2 subsequent siblings)
3 siblings, 1 reply; 6+ messages in thread
From: KP Singh @ 2021-01-12 13:14 UTC (permalink / raw)
To: Brendan Jackman
Cc: bpf, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Florent Revest, open list
On Tue, Jan 12, 2021 at 1:39 PM Brendan Jackman <jackmanb@google.com> wrote:
>
> The error message here is misleading, the argument will be rejected
> unless it is a known constant.
>
> Signed-off-by: Brendan Jackman <jackmanb@google.com>
> ---
> kernel/bpf/verifier.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 17270b8404f1..5534e667bdb1 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -4319,7 +4319,7 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg,
> err = mark_chain_precision(env, regno);
> } else if (arg_type_is_alloc_size(arg_type)) {
> if (!tnum_is_const(reg->var_off)) {
> - verbose(env, "R%d unbounded size, use 'var &= const' or 'if (var < const)'\n",
Can you check if:
int var = 1000;
var += 1;
if (var < 2000)
// call helper
and then using var in the argument works? If so, the existing error
message would be correct.
> + verbose(env, "R%d is not a known constant'\n",
> regno);
> return -EACCES;
> }
>
> base-commit: e22d7f05e445165e58feddb4e40cc9c0f94453bc
> --
> 2.30.0.284.gd98b1dd5eaa7-goog
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH bpf-next] bpf: Fix a verifier message for alloc size helper arg
2021-01-12 13:14 ` KP Singh
@ 2021-01-12 14:55 ` Brendan Jackman
0 siblings, 0 replies; 6+ messages in thread
From: Brendan Jackman @ 2021-01-12 14:55 UTC (permalink / raw)
To: KP Singh
Cc: bpf, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Florent Revest, open list
Sorry, duplicate - seems I had my mail client in HTML mode the first
time around.
On Tue, 12 Jan 2021 at 14:14, KP Singh <kpsingh@kernel.org> wrote:
>
> On Tue, Jan 12, 2021 at 1:39 PM Brendan Jackman <jackmanb@google.com> wrote:
> >
> > The error message here is misleading, the argument will be rejected
> > unless it is a known constant.
> >
> > Signed-off-by: Brendan Jackman <jackmanb@google.com>
> > ---
> > kernel/bpf/verifier.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> > index 17270b8404f1..5534e667bdb1 100644
> > --- a/kernel/bpf/verifier.c
> > +++ b/kernel/bpf/verifier.c
> > @@ -4319,7 +4319,7 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg,
> > err = mark_chain_precision(env, regno);
> > } else if (arg_type_is_alloc_size(arg_type)) {
> > if (!tnum_is_const(reg->var_off)) {
> > - verbose(env, "R%d unbounded size, use 'var &= const' or 'if (var < const)'\n",
>
> Can you check if:
>
> int var = 1000;
> var += 1;
>
> if (var < 2000)
> // call helper
>
> and then using var in the argument works? If so, the existing error
> message would be correct.
I think that would work because var is already a known constant before
the conditional.. but the error message is still wrong, the `if (var <
2000)` is irrelevant. If var was not already a known constant (e.g.
came from the return value of a bpf_probe_read_kernel_str) it would
fail verification.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH bpf-next] bpf: Fix a verifier message for alloc size helper arg
2021-01-12 12:39 [PATCH bpf-next] bpf: Fix a verifier message for alloc size helper arg Brendan Jackman
2021-01-12 13:14 ` KP Singh
@ 2021-01-12 16:42 ` Yonghong Song
2021-01-12 19:16 ` Andrii Nakryiko
2021-01-12 20:50 ` patchwork-bot+netdevbpf
3 siblings, 0 replies; 6+ messages in thread
From: Yonghong Song @ 2021-01-12 16:42 UTC (permalink / raw)
To: Brendan Jackman, bpf
Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, KP Singh,
Florent Revest, linux-kernel
On 1/12/21 4:39 AM, Brendan Jackman wrote:
> The error message here is misleading, the argument will be rejected
> unless it is a known constant.
>
> Signed-off-by: Brendan Jackman <jackmanb@google.com>
Okay, this is for bpf_ringbuf_reserve() helper where the size must be a
constant.
Acked-by: Yonghong Song <yhs@fb.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH bpf-next] bpf: Fix a verifier message for alloc size helper arg
2021-01-12 12:39 [PATCH bpf-next] bpf: Fix a verifier message for alloc size helper arg Brendan Jackman
2021-01-12 13:14 ` KP Singh
2021-01-12 16:42 ` Yonghong Song
@ 2021-01-12 19:16 ` Andrii Nakryiko
2021-01-12 20:50 ` patchwork-bot+netdevbpf
3 siblings, 0 replies; 6+ messages in thread
From: Andrii Nakryiko @ 2021-01-12 19:16 UTC (permalink / raw)
To: Brendan Jackman
Cc: bpf, Alexei Starovoitov, Daniel Borkmann, KP Singh,
Florent Revest, open list
On Tue, Jan 12, 2021 at 4:39 AM Brendan Jackman <jackmanb@google.com> wrote:
>
> The error message here is misleading, the argument will be rejected
> unless it is a known constant.
>
> Signed-off-by: Brendan Jackman <jackmanb@google.com>
> ---
LGTM.
Acked-by: Andrii Nakryiko <andrii@kernel.org>
> kernel/bpf/verifier.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 17270b8404f1..5534e667bdb1 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -4319,7 +4319,7 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg,
> err = mark_chain_precision(env, regno);
> } else if (arg_type_is_alloc_size(arg_type)) {
> if (!tnum_is_const(reg->var_off)) {
> - verbose(env, "R%d unbounded size, use 'var &= const' or 'if (var < const)'\n",
> + verbose(env, "R%d is not a known constant'\n",
> regno);
> return -EACCES;
> }
>
> base-commit: e22d7f05e445165e58feddb4e40cc9c0f94453bc
> --
> 2.30.0.284.gd98b1dd5eaa7-goog
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH bpf-next] bpf: Fix a verifier message for alloc size helper arg
2021-01-12 12:39 [PATCH bpf-next] bpf: Fix a verifier message for alloc size helper arg Brendan Jackman
` (2 preceding siblings ...)
2021-01-12 19:16 ` Andrii Nakryiko
@ 2021-01-12 20:50 ` patchwork-bot+netdevbpf
3 siblings, 0 replies; 6+ messages in thread
From: patchwork-bot+netdevbpf @ 2021-01-12 20:50 UTC (permalink / raw)
To: Brendan Jackman
Cc: bpf, ast, daniel, andrii.nakryiko, kpsingh, revest, linux-kernel
Hello:
This patch was applied to bpf/bpf-next.git (refs/heads/master):
On Tue, 12 Jan 2021 12:39:13 +0000 you wrote:
> The error message here is misleading, the argument will be rejected
> unless it is a known constant.
>
> Signed-off-by: Brendan Jackman <jackmanb@google.com>
> ---
> kernel/bpf/verifier.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> [...]
Here is the summary with links:
- [bpf-next] bpf: Fix a verifier message for alloc size helper arg
https://git.kernel.org/bpf/bpf-next/c/28a8add64181
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 6+ messages in thread