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

[Konstantin Ananyev <konstantin.ananyev@huawei.com>]

> 
> > > 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

Shouldn't it be UINT32_MAX?

> > > 	}
> > >
> > > 	// ...
> > > }

I wonder does x86 jit and VM perform that check?
If not, then it is probably not fair to demand that patch to fix these things for arm:
It probably has to be another patch that will fix that issue for all targets (vm, x86, arm).
After all - it wouldn't cause any memory corruption, right?
In the worst case (wraparound) bpf will read some valid data from the
unexpected location in the packet instead of simply returning 0.
BTW, I think we still need to add check for overflow in __rte_pktmbuf_read() -
bpf jit is not the only user of it. 
Unless, of-course, we want rte_pktmbuf_read() to work with wrapped-around values. 

> >
> > I don't know if it's the sum of src+imm that determines special meaning, or it's
> the imm itself.
> > If it's the imm itself, a simple fix would be to update the validator's .imm.max
> values for BPF_IND
> > from UINT32_MAX to INT32_MAX.
> >
> 
> Even if we do it (I have no particular opinion), it won't prevent sum of 64-bit
> register and immediate from being negative or greater than the range of values
> supported by 32-bit argument of __rte_pktmbuf_read.