More strict error checking in bpf_asm.
Subsystems (3)
cover More strict error checking in bpf_asm.
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.
tools/bpf/bpf_exp.y | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-)
--
2.30.1
PATCH 1/2 tools, bpf_asm: Hard error on out of range jumps.
Per discussion at:
https://lore.kernel.org/bpf/[email protected]/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(-)
tools/bpf/bpf_exp.y +4 -2
@@ -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; }
Signature · 1 line
2.30.1
PATCH 2/2 tools, bpf_asm: exit non-zero on errors.
In reply to Ian Denhardt...so callers can correctly detect failure.
--- tools/bpf/bpf_exp.y | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
tools/bpf/bpf_exp.y +4 -4
@@ -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;
Signature · 1 line
2.30.1
Re: [PATCH 1/2] tools, bpf_asm: Hard error on out of range jumps.
In reply to Ian DenhardtQuoted 22 lines · Ian Denhardt
Per discussion at:
https://lore.kernel.org/bpf/[email protected]/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(-)tools/bpf/bpf_exp.y +0
-0@@ -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, "
Quoted 8 lines
+ 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;
}
Daniel Borkmann
maintainerHi Ian,
Quoted 7 lines · Ian Denhardt
Hi,
Enclosed are two patches related to my earlier message, which make the error checking in the
bpf_asmtool more strict, the first by upgrading a warning to an error, the second by using non-zero exit codes when aborting.
Quoted 8 lines
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
Quoted 6 lines
tools/bpf/bpf_exp.y | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-)--
2.30.1