RE: [PATCH] bpf/arm64: support packet data load instructions

[Marat Khalili <marat.khalili@huawei.com>]

> > In Classic BPF, negative "k" has special meaning for both BPF_ABS and BPF_IND.
> > So we should consider it invalid for both cases.
> >
> > That prevents applications from using it the way you describe.
> > And it will allow us to add BPF library support for Linux-compatible special meanings later, without
> breaking the ABI.
> >
> 
> Aren't these invalid offsets already taken care during the syntax
> check when we validate the BPF program ?
> in bpf_validate.c +1499:
>         /* load absolute instructions */
>         [(BPF_LD | BPF_ABS | BPF_B)] = {
>                 .mask = {. dreg = ZERO_REG, .sreg = ZERO_REG},
>                 .off = { .min = 0, .max = 0},
>                 .imm = { .min = 0, .max = INT32_MAX},
>                 .eval = eval_ld_mbuf,
>         },
> 
> IIUC, as __rte_bpf_validate fails when we cal rte_bpf_load ( in
> bpf_load.c +113), we can't even interpret the cBPF program.

Good point, we can probably consider BPF_ABS case covered by this.

For BPF_IND however it does not seem to exclude any values, and even if it did
we don't know what's in the register. Speaking of which, I just noticed that
we're truncating it.

I suggest the following logic in pseudo-code:

static void 
emit_ld_mbuf(struct a64_jit_ctx *ctx, uint32_t op, uint8_t tmp1, uint8_t tmp2, 
	uint8_t src, int32_t imm)
{
	// ...

	/* r1 = off: for ABS use imm, for IND use src + imm */
	if (mode == BPF_ABS) {
		assert imm >= 0, "verified by verifier"
		emit MOV W1, #<imm>
	} else {
		/* add signed imm to the source register */
		emit(s) X1 = src + #<imm>
		/* verify dynamically that offset is within the domain of __rte_pktmbuf_read */
		emit(s) jump_to_epilogue if X1 <s 0 || X1 > INT32_MAX
	}

	// ...
}