* [PATCH 1/2] tools, bpf_asm: Hard error on out of range jumps.
2021-02-24 2:36 [PATCH 0/2] More strict error checking in bpf_asm Ian Denhardt
@ 2021-02-24 2:15 ` Ian Denhardt
2021-02-24 9:30 ` Ilya Leoshkevich
2021-02-24 2:24 ` [PATCH 2/2] tools, bpf_asm: exit non-zero on errors Ian Denhardt
2021-02-24 20:34 ` [PATCH 0/2] More strict error checking in bpf_asm Daniel Borkmann
2 siblings, 1 reply; 5+ messages in thread
From: Ian Denhardt @ 2021-02-24 2:15 UTC (permalink / raw)
To: ast, daniel, bpf, netdev
Per discussion at:
https://lore.kernel.org/bpf/c964892195a6b91d20a67691448567ef528ffa6d.camel@linux.ibm.com/T/#t
...this was originally introduced as a warning due to concerns about
breaking existing code, but a hard error probably makes more sense,
especially given that concerns about breakage were only speculation.
---
tools/bpf/bpf_exp.y | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/tools/bpf/bpf_exp.y b/tools/bpf/bpf_exp.y
index 8d48e896be50..8d03e5245da5 100644
--- a/tools/bpf/bpf_exp.y
+++ b/tools/bpf/bpf_exp.y
@@ -549,9 +549,11 @@ static uint8_t bpf_encode_jt_jf_offset(int off, int i)
{
int delta = off - i - 1;
- if (delta < 0 || delta > 255)
- fprintf(stderr, "warning: insn #%d jumps to insn #%d, "
+ if (delta < 0 || delta > 255) {
+ fprintf(stderr, "error: insn #%d jumps to insn #%d, "
"which is out of range\n", i, off);
+ exit(1);
+ }
return (uint8_t) delta;
}
--
2.30.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] tools, bpf_asm: Hard error on out of range jumps.
2021-02-24 2:15 ` [PATCH 1/2] tools, bpf_asm: Hard error on out of range jumps Ian Denhardt
@ 2021-02-24 9:30 ` Ilya Leoshkevich
0 siblings, 0 replies; 5+ messages in thread
From: Ilya Leoshkevich @ 2021-02-24 9:30 UTC (permalink / raw)
To: Ian Denhardt, ast, daniel, bpf, netdev
On Tue, 2021-02-23 at 21:15 -0500, Ian Denhardt wrote:
> Per discussion at:
>
> https://lore.kernel.org/bpf/c964892195a6b91d20a67691448567ef528ffa6d.camel@linux.ibm.com/T/#t
>
> ...this was originally introduced as a warning due to concerns about
> breaking existing code, but a hard error probably makes more sense,
> especially given that concerns about breakage were only speculation.
> ---
> tools/bpf/bpf_exp.y | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/tools/bpf/bpf_exp.y b/tools/bpf/bpf_exp.y
> index 8d48e896be50..8d03e5245da5 100644
> --- a/tools/bpf/bpf_exp.y
> +++ b/tools/bpf/bpf_exp.y
> @@ -549,9 +549,11 @@ static uint8_t bpf_encode_jt_jf_offset(int off,
> int i)
> {
> int delta = off - i - 1;
>
> - if (delta < 0 || delta > 255)
> - fprintf(stderr, "warning: insn #%d jumps to insn #%d, "
> + if (delta < 0 || delta > 255) {
> + fprintf(stderr, "error: insn #%d jumps to insn #%d, "
> "which is out of range\n", i, off);
> + exit(1);
> + }
> return (uint8_t) delta;
> }
>
Acked-by: Ilya Leoshkevich <iii@linux.ibm.com>
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 2/2] tools, bpf_asm: exit non-zero on errors.
2021-02-24 2:36 [PATCH 0/2] More strict error checking in bpf_asm Ian Denhardt
2021-02-24 2:15 ` [PATCH 1/2] tools, bpf_asm: Hard error on out of range jumps Ian Denhardt
@ 2021-02-24 2:24 ` Ian Denhardt
2021-02-24 20:34 ` [PATCH 0/2] More strict error checking in bpf_asm Daniel Borkmann
2 siblings, 0 replies; 5+ messages in thread
From: Ian Denhardt @ 2021-02-24 2:24 UTC (permalink / raw)
To: ast, daniel, bpf, netdev
...so callers can correctly detect failure.
---
tools/bpf/bpf_exp.y | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/tools/bpf/bpf_exp.y b/tools/bpf/bpf_exp.y
index 8d03e5245da5..dfb7254a24e8 100644
--- a/tools/bpf/bpf_exp.y
+++ b/tools/bpf/bpf_exp.y
@@ -185,13 +185,13 @@ ldx
| OP_LDXB number '*' '(' '[' number ']' '&' number ')' {
if ($2 != 4 || $9 != 0xf) {
fprintf(stderr, "ldxb offset not supported!\n");
- exit(0);
+ exit(1);
} else {
bpf_set_curr_instr(BPF_LDX | BPF_MSH | BPF_B, 0, 0, $6); } }
| OP_LDX number '*' '(' '[' number ']' '&' number ')' {
if ($2 != 4 || $9 != 0xf) {
fprintf(stderr, "ldxb offset not supported!\n");
- exit(0);
+ exit(1);
} else {
bpf_set_curr_instr(BPF_LDX | BPF_MSH | BPF_B, 0, 0, $6); } }
;
@@ -472,7 +472,7 @@ static void bpf_assert_max(void)
{
if (curr_instr >= BPF_MAXINSNS) {
fprintf(stderr, "only max %u insns allowed!\n", BPF_MAXINSNS);
- exit(0);
+ exit(1);
}
}
@@ -522,7 +522,7 @@ static int bpf_find_insns_offset(const char *label)
if (ret == -ENOENT) {
fprintf(stderr, "no such label \'%s\'!\n", label);
- exit(0);
+ exit(1);
}
return ret;
--
2.30.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 0/2] More strict error checking in bpf_asm.
2021-02-24 2:36 [PATCH 0/2] More strict error checking in bpf_asm Ian Denhardt
2021-02-24 2:15 ` [PATCH 1/2] tools, bpf_asm: Hard error on out of range jumps Ian Denhardt
2021-02-24 2:24 ` [PATCH 2/2] tools, bpf_asm: exit non-zero on errors Ian Denhardt
@ 2021-02-24 20:34 ` Daniel Borkmann
2 siblings, 0 replies; 5+ messages in thread
From: Daniel Borkmann @ 2021-02-24 20:34 UTC (permalink / raw)
To: Ian Denhardt, ast, bpf, netdev
Hi Ian,
On 2/24/21 3:36 AM, Ian Denhardt wrote:
> Hi,
>
> Enclosed are two patches related to my earlier message, which make the
> error checking in the bpf_asm tool more strict, the first by upgrading a
> warning to an error, the second by using non-zero exit codes when
> aborting.
>
> These could be conceptually separated, but it seemed sensible to submit
> them together.
>
> -Ian
>
> Ian Denhardt (2):
> tools, bpf_asm: Hard error on out of range jumps.
> tools, bpf_asm: exit non-zero on errors.
Both of the patches need to have your Signed-off-by [0] in order to be able
to apply them, for example see [1]. Please resubmit with them & feel free to
carry Ilya's ACK forward for the v2. Thanks!
[0] https://www.kernel.org/doc/html/latest/process/submitting-patches.html
[1] https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf.git/commit/?id=557c223b643a35effec9654958d8edc62fd2603a
> tools/bpf/bpf_exp.y | 14 ++++++++------
> 1 file changed, 8 insertions(+), 6 deletions(-)
>
> --
> 2.30.1
>
^ permalink raw reply [flat|nested] 5+ messages in thread