> > > 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.
Agree.
>
> 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
> }
>
> // ...
> }
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.
