BPF Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH bpf-next] bpf: Fix a verifier message for alloc size helper arg
@ 2021-01-12 12:39 Brendan Jackman
  2021-01-12 13:14 ` KP Singh
                   ` (3 more replies)
  0 siblings, 4 replies; 6+ messages in thread
From: Brendan Jackman @ 2021-01-12 12:39 UTC (permalink / raw)
  To: bpf
  Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, KP Singh,
	Florent Revest, linux-kernel, Brendan Jackman

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",
+			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
@ 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

end of thread, back to index

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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
2021-01-12 19:16 ` Andrii Nakryiko
2021-01-12 20:50 ` patchwork-bot+netdevbpf

BPF Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/bpf/0 bpf/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 bpf bpf/ https://lore.kernel.org/bpf \
		bpf@vger.kernel.org
	public-inbox-index bpf

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.bpf


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git