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

bpf: Simplify reg_set_min_max_inv handling

reg_set_min_max_inv() contains exactly the same logic as reg_set_min_max(),
just flipped around. While this makes sense in a cBPF verifier (where ALU
operations are not symmetric), it does not make sense for eBPF.

Replace reg_set_min_max_inv() with a helper that flips the opcode around,
then lets reg_set_min_max() do the complicated work.

Signed-off-by: Jann Horn <jannh@google.com>
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Link: https://lore.kernel.org/bpf/20200330160324.15259-4-daniel@iogearbox.net

authored by

Jann Horn and committed by
Alexei Starovoitov
0fc31b10 604dca5e

+22 -86
+22 -86
kernel/bpf/verifier.c
··· 5836 5836 break; 5837 5837 } 5838 5838 default: 5839 - break; 5839 + return; 5840 5840 } 5841 5841 5842 5842 __reg_deduce_bounds(false_reg); ··· 5859 5859 struct bpf_reg_state *false_reg, u64 val, 5860 5860 u8 opcode, bool is_jmp32) 5861 5861 { 5862 - s64 sval; 5863 - 5864 - if (__is_pointer_value(false, false_reg)) 5865 - return; 5866 - 5867 - val = is_jmp32 ? (u32)val : val; 5868 - sval = is_jmp32 ? (s64)(s32)val : (s64)val; 5869 - 5870 - switch (opcode) { 5871 - case BPF_JEQ: 5872 - case BPF_JNE: 5873 - { 5874 - struct bpf_reg_state *reg = 5875 - opcode == BPF_JEQ ? true_reg : false_reg; 5876 - 5877 - if (is_jmp32) { 5878 - u64 old_v = reg->var_off.value; 5879 - u64 hi_mask = ~0xffffffffULL; 5880 - 5881 - reg->var_off.value = (old_v & hi_mask) | val; 5882 - reg->var_off.mask &= hi_mask; 5883 - } else { 5884 - __mark_reg_known(reg, val); 5885 - } 5886 - break; 5887 - } 5888 - case BPF_JSET: 5889 - false_reg->var_off = tnum_and(false_reg->var_off, 5890 - tnum_const(~val)); 5891 - if (is_power_of_2(val)) 5892 - true_reg->var_off = tnum_or(true_reg->var_off, 5893 - tnum_const(val)); 5894 - break; 5895 - case BPF_JGE: 5896 - case BPF_JGT: 5897 - { 5898 - set_lower_bound(false_reg, val, is_jmp32, opcode == BPF_JGE); 5899 - set_upper_bound(true_reg, val, is_jmp32, opcode == BPF_JGT); 5900 - break; 5901 - } 5902 - case BPF_JSGE: 5903 - case BPF_JSGT: 5904 - { 5905 - s64 false_smin = opcode == BPF_JSGT ? sval : sval + 1; 5906 - s64 true_smax = opcode == BPF_JSGT ? sval - 1 : sval; 5907 - 5908 - if (is_jmp32 && !cmp_val_with_extended_s64(sval, false_reg)) 5909 - break; 5910 - false_reg->smin_value = max(false_reg->smin_value, false_smin); 5911 - true_reg->smax_value = min(true_reg->smax_value, true_smax); 5912 - break; 5913 - } 5914 - case BPF_JLE: 5915 - case BPF_JLT: 5916 - { 5917 - set_upper_bound(false_reg, val, is_jmp32, opcode == BPF_JLE); 5918 - set_lower_bound(true_reg, val, is_jmp32, opcode == BPF_JLT); 5919 - break; 5920 - } 5921 - case BPF_JSLE: 5922 - case BPF_JSLT: 5923 - { 5924 - s64 false_smax = opcode == BPF_JSLT ? sval : sval - 1; 5925 - s64 true_smin = opcode == BPF_JSLT ? sval + 1 : sval; 5926 - 5927 - if (is_jmp32 && !cmp_val_with_extended_s64(sval, false_reg)) 5928 - break; 5929 - false_reg->smax_value = min(false_reg->smax_value, false_smax); 5930 - true_reg->smin_value = max(true_reg->smin_value, true_smin); 5931 - break; 5932 - } 5933 - default: 5934 - break; 5935 - } 5936 - 5937 - __reg_deduce_bounds(false_reg); 5938 - __reg_deduce_bounds(true_reg); 5939 - /* We might have learned some bits from the bounds. */ 5940 - __reg_bound_offset(false_reg); 5941 - __reg_bound_offset(true_reg); 5942 - /* Intersecting with the old var_off might have improved our bounds 5943 - * slightly. e.g. if umax was 0x7f...f and var_off was (0; 0xf...fc), 5944 - * then new var_off is (0; 0x7f...fc) which improves our umax. 5862 + /* How can we transform "a <op> b" into "b <op> a"? */ 5863 + static const u8 opcode_flip[16] = { 5864 + /* these stay the same */ 5865 + [BPF_JEQ >> 4] = BPF_JEQ, 5866 + [BPF_JNE >> 4] = BPF_JNE, 5867 + [BPF_JSET >> 4] = BPF_JSET, 5868 + /* these swap "lesser" and "greater" (L and G in the opcodes) */ 5869 + [BPF_JGE >> 4] = BPF_JLE, 5870 + [BPF_JGT >> 4] = BPF_JLT, 5871 + [BPF_JLE >> 4] = BPF_JGE, 5872 + [BPF_JLT >> 4] = BPF_JGT, 5873 + [BPF_JSGE >> 4] = BPF_JSLE, 5874 + [BPF_JSGT >> 4] = BPF_JSLT, 5875 + [BPF_JSLE >> 4] = BPF_JSGE, 5876 + [BPF_JSLT >> 4] = BPF_JSGT 5877 + }; 5878 + opcode = opcode_flip[opcode >> 4]; 5879 + /* This uses zero as "not present in table"; luckily the zero opcode, 5880 + * BPF_JA, can't get here. 5945 5881 */ 5946 - __update_reg_bounds(false_reg); 5947 - __update_reg_bounds(true_reg); 5882 + if (opcode) 5883 + reg_set_min_max(true_reg, false_reg, val, opcode, is_jmp32); 5948 5884 } 5949 5885 5950 5886 /* Regs are known to be equal, so intersect their min/max/var_off */