Skip to content

Commit

Permalink
bpf, x32: Fix bug with ALU64 {LSH, RSH, ARSH} BPF_K shift by 0
Browse files Browse the repository at this point in the history
The current x32 BPF JIT does not correctly compile shift operations when
the immediate shift amount is 0. The expected behavior is for this to
be a no-op.

The following program demonstrates the bug. The expexceted result is 1,
but the current JITed code returns 2.

  r0 = 1
  r1 = 1
  r1 <<= 0
  if r1 == 1 goto end
  r0 = 2
end:
  exit

This patch simplifies the code and fixes the bug.

Fixes: 03f5781 ("bpf, x86_32: add eBPF JIT compiler for ia32")
Co-developed-by: Xi Wang <xi.wang@gmail.com>
Signed-off-by: Xi Wang <xi.wang@gmail.com>
Signed-off-by: Luke Nelson <luke.r.nels@gmail.com>
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
  • Loading branch information
Luke Nelson authored and Daniel Borkmann committed Jul 3, 2019
1 parent 68a8357 commit 6fa632e
Showing 1 changed file with 6 additions and 57 deletions.
63 changes: 6 additions & 57 deletions arch/x86/net/bpf_jit_comp32.c
Original file line number Diff line number Diff line change
Expand Up @@ -894,27 +894,10 @@ static inline void emit_ia32_lsh_i64(const u8 dst[], const u32 val,
}
/* Do LSH operation */
if (val < 32) {
/* shl dreg_hi,imm8 */
EMIT3(0xC1, add_1reg(0xE0, dreg_hi), val);
/* mov ebx,dreg_lo */
EMIT2(0x8B, add_2reg(0xC0, dreg_lo, IA32_EBX));
/* shld dreg_hi,dreg_lo,imm8 */
EMIT4(0x0F, 0xA4, add_2reg(0xC0, dreg_hi, dreg_lo), val);
/* shl dreg_lo,imm8 */
EMIT3(0xC1, add_1reg(0xE0, dreg_lo), val);

/* IA32_ECX = 32 - val */
/* mov ecx,val */
EMIT2(0xB1, val);
/* movzx ecx,ecx */
EMIT3(0x0F, 0xB6, add_2reg(0xC0, IA32_ECX, IA32_ECX));
/* neg ecx */
EMIT2(0xF7, add_1reg(0xD8, IA32_ECX));
/* add ecx,32 */
EMIT3(0x83, add_1reg(0xC0, IA32_ECX), 32);

/* shr ebx,cl */
EMIT2(0xD3, add_1reg(0xE8, IA32_EBX));
/* or dreg_hi,ebx */
EMIT2(0x09, add_2reg(0xC0, dreg_hi, IA32_EBX));
} else if (val >= 32 && val < 64) {
u32 value = val - 32;

Expand Down Expand Up @@ -960,27 +943,10 @@ static inline void emit_ia32_rsh_i64(const u8 dst[], const u32 val,

/* Do RSH operation */
if (val < 32) {
/* shr dreg_lo,imm8 */
EMIT3(0xC1, add_1reg(0xE8, dreg_lo), val);
/* mov ebx,dreg_hi */
EMIT2(0x8B, add_2reg(0xC0, dreg_hi, IA32_EBX));
/* shrd dreg_lo,dreg_hi,imm8 */
EMIT4(0x0F, 0xAC, add_2reg(0xC0, dreg_lo, dreg_hi), val);
/* shr dreg_hi,imm8 */
EMIT3(0xC1, add_1reg(0xE8, dreg_hi), val);

/* IA32_ECX = 32 - val */
/* mov ecx,val */
EMIT2(0xB1, val);
/* movzx ecx,ecx */
EMIT3(0x0F, 0xB6, add_2reg(0xC0, IA32_ECX, IA32_ECX));
/* neg ecx */
EMIT2(0xF7, add_1reg(0xD8, IA32_ECX));
/* add ecx,32 */
EMIT3(0x83, add_1reg(0xC0, IA32_ECX), 32);

/* shl ebx,cl */
EMIT2(0xD3, add_1reg(0xE0, IA32_EBX));
/* or dreg_lo,ebx */
EMIT2(0x09, add_2reg(0xC0, dreg_lo, IA32_EBX));
} else if (val >= 32 && val < 64) {
u32 value = val - 32;

Expand Down Expand Up @@ -1025,27 +991,10 @@ static inline void emit_ia32_arsh_i64(const u8 dst[], const u32 val,
}
/* Do RSH operation */
if (val < 32) {
/* shr dreg_lo,imm8 */
EMIT3(0xC1, add_1reg(0xE8, dreg_lo), val);
/* mov ebx,dreg_hi */
EMIT2(0x8B, add_2reg(0xC0, dreg_hi, IA32_EBX));
/* shrd dreg_lo,dreg_hi,imm8 */
EMIT4(0x0F, 0xAC, add_2reg(0xC0, dreg_lo, dreg_hi), val);
/* ashr dreg_hi,imm8 */
EMIT3(0xC1, add_1reg(0xF8, dreg_hi), val);

/* IA32_ECX = 32 - val */
/* mov ecx,val */
EMIT2(0xB1, val);
/* movzx ecx,ecx */
EMIT3(0x0F, 0xB6, add_2reg(0xC0, IA32_ECX, IA32_ECX));
/* neg ecx */
EMIT2(0xF7, add_1reg(0xD8, IA32_ECX));
/* add ecx,32 */
EMIT3(0x83, add_1reg(0xC0, IA32_ECX), 32);

/* shl ebx,cl */
EMIT2(0xD3, add_1reg(0xE0, IA32_EBX));
/* or dreg_lo,ebx */
EMIT2(0x09, add_2reg(0xC0, dreg_lo, IA32_EBX));
} else if (val >= 32 && val < 64) {
u32 value = val - 32;

Expand Down

0 comments on commit 6fa632e

Please sign in to comment.