2021-01-12 13:10:32

by Brendan Jackman

[permalink] [raw]
Subject: [PATCH bpf-next] bpf: Fix a verifier message for alloc size helper arg

The error message here is misleading, the argument will be rejected
unless it is a known constant.

Signed-off-by: Brendan Jackman <[email protected]>
---
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


2021-01-12 14:59:41

by Brendan Jackman

[permalink] [raw]
Subject: Re: [PATCH bpf-next] bpf: Fix a verifier message for alloc size helper arg

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 <[email protected]> wrote:
>
> On Tue, Jan 12, 2021 at 1:39 PM Brendan Jackman <[email protected]> wrote:
> >
> > The error message here is misleading, the argument will be rejected
> > unless it is a known constant.
> >
> > Signed-off-by: Brendan Jackman <[email protected]>
> > ---
> > 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.

2021-01-12 16:45:57

by Yonghong Song

[permalink] [raw]
Subject: Re: [PATCH bpf-next] bpf: Fix a verifier message for alloc size helper arg



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 <[email protected]>

Okay, this is for bpf_ringbuf_reserve() helper where the size must be a
constant.

Acked-by: Yonghong Song <[email protected]>

2021-01-13 01:43:24

by KP Singh

[permalink] [raw]
Subject: Re: [PATCH bpf-next] bpf: Fix a verifier message for alloc size helper arg

On Tue, Jan 12, 2021 at 1:39 PM Brendan Jackman <[email protected]> wrote:
>
> The error message here is misleading, the argument will be rejected
> unless it is a known constant.
>
> Signed-off-by: Brendan Jackman <[email protected]>
> ---
> 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
>

2021-01-13 02:27:10

by Andrii Nakryiko

[permalink] [raw]
Subject: Re: [PATCH bpf-next] bpf: Fix a verifier message for alloc size helper arg

On Tue, Jan 12, 2021 at 4:39 AM Brendan Jackman <[email protected]> wrote:
>
> The error message here is misleading, the argument will be rejected
> unless it is a known constant.
>
> Signed-off-by: Brendan Jackman <[email protected]>
> ---

LGTM.

Acked-by: Andrii Nakryiko <[email protected]>

> 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
>

2021-01-13 02:36:32

by patchwork-bot+netdevbpf

[permalink] [raw]
Subject: Re: [PATCH bpf-next] bpf: Fix a verifier message for alloc size helper arg

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 <[email protected]>
> ---
> 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