| Message ID | 20210112123913.2016804-1-jackmanb@google.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 28a8add64181059034b7f281491132112cd95bb4 |
| Delegated to: | BPF |
| Headers | show |
| Series | [bpf-next] bpf: Fix a verifier message for alloc size helper arg | expand |
| Context | Check | Description |
|---|---|---|
| netdev/cover_letter | success | Link |
| netdev/fixes_present | success | Link |
| netdev/patch_count | success | Link |
| netdev/tree_selection | success | Clearly marked for bpf-next |
| netdev/subject_prefix | success | Link |
| netdev/cc_maintainers | warning | 7 maintainers not CCed: kafai@fb.com netdev@vger.kernel.org yhs@fb.com kpsingh@kernel.org andrii@kernel.org songliubraving@fb.com john.fastabend@gmail.com |
| netdev/source_inline | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Link |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/build_32bit | success | Errors and warnings before: 17 this patch: 17 |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/verify_fixes | success | Link |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 8 lines checked |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 17 this patch: 17 |
| netdev/header_inline | success | Link |
| netdev/stable | success | Stable not CCed |
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 >
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.
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>
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 >
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
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; }
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(-) base-commit: e22d7f05e445165e58feddb4e40cc9c0f94453bc