Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

bpf, x32: Fix bug with ALU64 {LSH, RSH, ARSH} BPF_K shift by 0

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: 03f5781be2c7 ("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>

authored by

Luke Nelson and committed by
Daniel Borkmann
6fa632e7 68a8357e

+6 -57
+6 -57
arch/x86/net/bpf_jit_comp32.c
··· 894 894 } 895 895 /* Do LSH operation */ 896 896 if (val < 32) { 897 - /* shl dreg_hi,imm8 */ 898 - EMIT3(0xC1, add_1reg(0xE0, dreg_hi), val); 899 - /* mov ebx,dreg_lo */ 900 - EMIT2(0x8B, add_2reg(0xC0, dreg_lo, IA32_EBX)); 897 + /* shld dreg_hi,dreg_lo,imm8 */ 898 + EMIT4(0x0F, 0xA4, add_2reg(0xC0, dreg_hi, dreg_lo), val); 901 899 /* shl dreg_lo,imm8 */ 902 900 EMIT3(0xC1, add_1reg(0xE0, dreg_lo), val); 903 - 904 - /* IA32_ECX = 32 - val */ 905 - /* mov ecx,val */ 906 - EMIT2(0xB1, val); 907 - /* movzx ecx,ecx */ 908 - EMIT3(0x0F, 0xB6, add_2reg(0xC0, IA32_ECX, IA32_ECX)); 909 - /* neg ecx */ 910 - EMIT2(0xF7, add_1reg(0xD8, IA32_ECX)); 911 - /* add ecx,32 */ 912 - EMIT3(0x83, add_1reg(0xC0, IA32_ECX), 32); 913 - 914 - /* shr ebx,cl */ 915 - EMIT2(0xD3, add_1reg(0xE8, IA32_EBX)); 916 - /* or dreg_hi,ebx */ 917 - EMIT2(0x09, add_2reg(0xC0, dreg_hi, IA32_EBX)); 918 901 } else if (val >= 32 && val < 64) { 919 902 u32 value = val - 32; 920 903 ··· 943 960 944 961 /* Do RSH operation */ 945 962 if (val < 32) { 946 - /* shr dreg_lo,imm8 */ 947 - EMIT3(0xC1, add_1reg(0xE8, dreg_lo), val); 948 - /* mov ebx,dreg_hi */ 949 - EMIT2(0x8B, add_2reg(0xC0, dreg_hi, IA32_EBX)); 963 + /* shrd dreg_lo,dreg_hi,imm8 */ 964 + EMIT4(0x0F, 0xAC, add_2reg(0xC0, dreg_lo, dreg_hi), val); 950 965 /* shr dreg_hi,imm8 */ 951 966 EMIT3(0xC1, add_1reg(0xE8, dreg_hi), val); 952 - 953 - /* IA32_ECX = 32 - val */ 954 - /* mov ecx,val */ 955 - EMIT2(0xB1, val); 956 - /* movzx ecx,ecx */ 957 - EMIT3(0x0F, 0xB6, add_2reg(0xC0, IA32_ECX, IA32_ECX)); 958 - /* neg ecx */ 959 - EMIT2(0xF7, add_1reg(0xD8, IA32_ECX)); 960 - /* add ecx,32 */ 961 - EMIT3(0x83, add_1reg(0xC0, IA32_ECX), 32); 962 - 963 - /* shl ebx,cl */ 964 - EMIT2(0xD3, add_1reg(0xE0, IA32_EBX)); 965 - /* or dreg_lo,ebx */ 966 - EMIT2(0x09, add_2reg(0xC0, dreg_lo, IA32_EBX)); 967 967 } else if (val >= 32 && val < 64) { 968 968 u32 value = val - 32; 969 969 ··· 991 1025 } 992 1026 /* Do RSH operation */ 993 1027 if (val < 32) { 994 - /* shr dreg_lo,imm8 */ 995 - EMIT3(0xC1, add_1reg(0xE8, dreg_lo), val); 996 - /* mov ebx,dreg_hi */ 997 - EMIT2(0x8B, add_2reg(0xC0, dreg_hi, IA32_EBX)); 1028 + /* shrd dreg_lo,dreg_hi,imm8 */ 1029 + EMIT4(0x0F, 0xAC, add_2reg(0xC0, dreg_lo, dreg_hi), val); 998 1030 /* ashr dreg_hi,imm8 */ 999 1031 EMIT3(0xC1, add_1reg(0xF8, dreg_hi), val); 1000 - 1001 - /* IA32_ECX = 32 - val */ 1002 - /* mov ecx,val */ 1003 - EMIT2(0xB1, val); 1004 - /* movzx ecx,ecx */ 1005 - EMIT3(0x0F, 0xB6, add_2reg(0xC0, IA32_ECX, IA32_ECX)); 1006 - /* neg ecx */ 1007 - EMIT2(0xF7, add_1reg(0xD8, IA32_ECX)); 1008 - /* add ecx,32 */ 1009 - EMIT3(0x83, add_1reg(0xC0, IA32_ECX), 32); 1010 - 1011 - /* shl ebx,cl */ 1012 - EMIT2(0xD3, add_1reg(0xE0, IA32_EBX)); 1013 - /* or dreg_lo,ebx */ 1014 - EMIT2(0x09, add_2reg(0xC0, dreg_lo, IA32_EBX)); 1015 1032 } else if (val >= 32 && val < 64) { 1016 1033 u32 value = val - 32; 1017 1034