Messages in this thread | | | Subject | Re: [PATCH bpf-next] Detect jumping to reserved code during check_cfg() | From | Daniel Borkmann <> | Date | Tue, 10 Oct 2023 17:35:05 +0200 |
| |
On 10/10/23 11:17 AM, Hao Sun wrote: [...] > I regard this as a fix, because the verifier log is not correct, since > the program does > not contain any invalid ld_imm64 instructions in this case. > > I haven't met other cases not captured via check_ld_imm(), but somehow, I think > we probably want to convert the check there as an internal bug, > because we already > have bpf_opcode_in_insntable() check in resolve_pseudo_ldimm64(). Once we meet > invalid insn code here, then somewhere else in the verifier is > probably wrong. But > I'm not sure, maybe something like this:
Makes sense, you could probably add this into your series as a separate commit.
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c > index eed7350e15f4..bed97de568a5 100644 > --- a/kernel/bpf/verifier.c > +++ b/kernel/bpf/verifier.c > @@ -14532,8 +14532,8 @@ static int check_ld_imm(struct > bpf_verifier_env *env, struct bpf_insn *insn) > int err; > > if (BPF_SIZE(insn->code) != BPF_DW) { > - verbose(env, "invalid BPF_LD_IMM insn\n"); > - return -EINVAL; > + verbose(env, "verifier internal bug, invalid BPF_LD_IMM\n");
If so please stick to the common style as we have in other locations:
verbose(env, "verifier internal error: <xyz>\n");
> + return -EFAULT; > } > if (insn->off != 0) { > verbose(env, "BPF_LD_IMM64 uses reserved fields\n"); >
| |