Classic BPF filters using a variable offset into the packet, such as "tcp[tcpflags] & tcp-syn != 0" or "tcp[0:4] = ...", read the wrong bytes. The indirect loads ldb/ld [x + k] were handled in the same case as the arithmetic instructions, which take the source register from BPF_SRC(code), that is bit 0x08. In a load opcode that bit is part of the size field, not a K/X selector, so BPF_LD|BPF_IND|BPF_B (0x50) and BPF_LD|BPF_IND|BPF_W (0x40) both yield BPF_K and the index register becomes R0 (A) instead of R7 (X). The halfword form works only because BPF_H and BPF_X are both 0x08. Give the indirect loads their own case next to the absolute loads and emit X as the index register. The interpreter and the JITs already take the index from src_reg, so this fixes all engines. Bugzilla ID: 2085 Fixes: 2eccf6afbea9 ("bpf: add function to convert classic BPF to DPDK BPF") Cc: [email protected] Signed-off-by: Stephen Hemminger <[email protected]> --- lib/bpf/bpf_convert.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/lib/bpf/bpf_convert.c b/lib/bpf/bpf_convert.c index 37095eea52..a32e1cdd8a 100644 --- a/lib/bpf/bpf_convert.c +++ b/lib/bpf/bpf_convert.c @@ -301,6 +301,13 @@ static int bpf_convert_filter(const struct bpf_insn *prog, size_t len, *insn = BPF_RAW_INSN(fp->code, 0, 0, 0, fp->k); break;+ /* Indirect loads index the packet with X: A = P[X + k] */+ case BPF_LD | BPF_IND | BPF_W: + case BPF_LD | BPF_IND | BPF_H: + case BPF_LD | BPF_IND | BPF_B: + *insn = BPF_RAW_INSN(fp->code, 0, BPF_REG_X, 0, fp->k); + break; + case BPF_ALU | BPF_DIV | BPF_X: case BPF_ALU | BPF_MOD | BPF_X: /* For cBPF, don't cause floating point exception */ @@ -328,9 +335,6 @@ static int bpf_convert_filter(const struct bpf_insn *prog, size_t len, case BPF_ALU | BPF_DIV | BPF_K: case BPF_ALU | BPF_MOD | BPF_K: case BPF_ALU | BPF_NEG: - case BPF_LD | BPF_IND | BPF_W: - case BPF_LD | BPF_IND | BPF_H: - case BPF_LD | BPF_IND | BPF_B: /* All arithmetic insns map as-is. */ insn->code = fp->code; insn->dst_reg = BPF_REG_A;
LGTM, one note: As Marat already mentioned, can we have a new test-case for it? Acked-by: Konstantin Ananyev <[email protected]>
