(I've looked at all 3 patches, and they seem fine... but I don't claim
to understand the intricacies of the verifier/registers/etc well
enough to be certain)
On Mon, Jan 8, 2024 at 5:28 AM Eduard Zingerman <eddyz87@xxxxxxxxx> wrote:
>
> Extend try_match_pkt_pointers() to handle == and != operations.
> For instruction:
>
> .--------------- pointer to packet with some range R
> | .--------- pointer to packet end
> v v
> if rA == rB goto ...
it's possible this would be better without the 'goto' as just 'if (rA
== rB) ...'
>
> It is valid to infer that R bytes are available in packet.
> This change should allow verification of BPF generated for
> C code like below:
>
> if (data + 42 != data_end) { ... }
this should probably be:
if (data + 42 != data_end) return;
...
>
> Suggested-by: Maciej Żenczykowski <zenczykowski@xxxxxxxxx>
> Link: https://lore.kernel.org/bpf/CAHo-Oow5V2u4ZYvzuR8NmJmFDPNYp0pQDJX66rZqUjFHvhx82A@xxxxxxxxxxxxxx/
> Signed-off-by: Eduard Zingerman <eddyz87@xxxxxxxxx>
> ---
> kernel/bpf/verifier.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 918e6a7912e2..b229ba0ad114 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -14677,6 +14677,7 @@ static bool try_match_pkt_pointers(const struct bpf_insn *insn,
> struct bpf_verifier_state *this_branch,
> struct bpf_verifier_state *other_branch)
> {
> + struct bpf_verifier_state *eq_branch;
> int opcode = BPF_OP(insn->code);
> int dst_regno = insn->dst_reg;
>
> @@ -14713,6 +14714,13 @@ static bool try_match_pkt_pointers(const struct bpf_insn *insn,
> find_good_pkt_pointers(other_branch, dst_reg, dst_reg->type, opcode == BPF_JLT);
> mark_pkt_end(this_branch, dst_regno, opcode == BPF_JLE);
> break;
> + case BPF_JEQ:
> + case BPF_JNE:
> + /* pkt_data ==/!= pkt_end, pkt_meta ==/!= pkt_data */
> + eq_branch = opcode == BPF_JEQ ? other_branch : this_branch;
> + find_good_pkt_pointers(eq_branch, dst_reg, dst_reg->type, true);
> + mark_pkt_end(eq_branch, dst_regno, false);
> + break;
> default:
> return false;
> }
> --
> 2.43.0
>
