Re: Funky verifier packet range error (> check works, != does not).

[Eduard Zingerman <eddyz87@gmail.com>]

On Tue, 2024-01-02 at 10:30 -0800, Maciej Żenczykowski wrote:
[...]
> 
> The check is:
>   if (data + 98 != data_end) return;
> so now (after this check) you *know* that 'data + 98 == data_end' and
> thus you know there are *exactly* 98 valid bytes.

Apologies, you are correct.
So you want to have something along the following lines:

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index a376eb609c41..6ddb34d5b9aa 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -14712,6 +14712,28 @@ static bool try_match_pkt_pointers(const struct bpf_insn *insn,
                        return false;
                }
                break;
+       case BPF_JEQ:
+       case BPF_JNE:
+               if ((dst_reg->type == PTR_TO_PACKET &&
+                    src_reg->type == PTR_TO_PACKET_END) ||
+                   (dst_reg->type == PTR_TO_PACKET_META &&
+                    reg_is_init_pkt_pointer(src_reg, PTR_TO_PACKET)) ||
+                   (dst_reg->type == PTR_TO_PACKET_END &&
+                    src_reg->type == PTR_TO_PACKET) ||
+                   (reg_is_init_pkt_pointer(dst_reg, PTR_TO_PACKET) &&
+                    src_reg->type == PTR_TO_PACKET_META)) {
+                       /* pkt_data' != pkt_end, pkt_meta' != pkt_data,
+                        * pkt_end != pkt_data', pkt_data != pkt_meta'
+                        */
+                       struct bpf_verifier_state *eq_branch;
+
+                       eq_branch = BPF_OP(insn->code) == BPF_JEQ ? other_branch : this_branch;
+                       find_good_pkt_pointers(eq_branch, dst_reg, dst_reg->type, true);
+                       mark_pkt_end(eq_branch, insn->dst_reg, false);
+               } else {
+                       return false;
+               }
+               break;
        case BPF_JGE:
                if ((dst_reg->type == PTR_TO_PACKET &&
                     src_reg->type == PTR_TO_PACKET_END) ||

Right?

This passes the following test case:

SEC("tc")
__success
__naked void data_plus_const_eq_pkt_end(void)
{
        asm volatile ("                                 \n\
        r9 = r1;                                        \n\
        r1 = *(u32*)(r9 + %[__sk_buff_data]);           \n\
        r2 = *(u32*)(r9 + %[__sk_buff_data_end]);       \n\
        r3 = r1;                                        \n\
        r3 += 8;                                        \n\
        if r3 != r2 goto 1f;                            \n\
        r1 = *(u64 *)(r1 + 0);                          \n\
1:                                                      \n\
        r0 = 0;                                         \n\
        exit;                                           \n\
"       :
        : __imm_const(__sk_buff_data, offsetof(struct __sk_buff, data)),
          __imm_const(__sk_buff_data_end, offsetof(struct __sk_buff, data_end))
        : __clobber_all);
}

And it's variations for EQ/NEQ, positive/negative.