Fix a bug in the verifier's scalar32_min_max_*() functions which leads to incorrect tracking of 32 bit bounds for the simulation of and/or/xor bitops. When both the src & dst subreg is a known constant, then the assumption is that scalar_min_max_*() will take care to update bounds correctly. However, this is not the case, for example, consider a register R2 which has a tnum of 0xffffffff00000000, meaning, lower 32 bits are known constant and in this case of value 0x00000001. R2 is then and'ed with a register R3 which is a 64 bit known constant, here, 0x100000002. What can be seen in line '10:' is that 32 bit bounds reach an invalid state where {u,s}32_min_value > {u,s}32_max_value. The reason is scalar32_min_max_*() delegates 32 bit bounds updates to scalar_min_max_*(), however, that really only takes place when both the 64 bit src & dst register is a known constant. Given scalar32_min_max_*() is intended to be designed as closely as possible to scalar_min_max_*(), update the 32 bit bounds in this situation through __mark_reg32_known() which will set all {u,s}32_{min,max}_value to the correct constant, which is 0x00000000 after the fix (given 0x00000001 & 0x00000002 in 32 bit space). This is possible given var32_off already holds the final value as dst_reg->var_off is updated before calling scalar32_min_max_*(). Before fix, invalid tracking of R2: [...] 9: R0_w=inv1337 R1=ctx(id=0,off=0,imm=0) R2_w=inv(id=0,smin_value=-9223372036854775807 (0x8000000000000001),smax_value=9223372032559808513 (0x7fffffff00000001),umin_value=1,umax_value=0xffffffff00000001,var_off=(0x1; 0xffffffff00000000),s32_min_value=1,s32_max_value=1,u32_min_value=1,u32_max_value=1) R3_w=inv4294967298 R10=fp0 9: (5f) r2 &= r3 10: R0_w=inv1337 R1=ctx(id=0,off=0,imm=0) R2_w=inv(id=0,smin_value=0,smax_value=4294967296 (0x100000000),umin_value=0,umax_value=0x100000000,var_off=(0x0; 0x100000000),s32_min_value=1,s32_max_value=0,u32_min_value=1,u32_max_value=0) R3_w=inv4294967298 R10=fp0 [...] After fix, correct tracking of R2: [...] 9: R0_w=inv1337 R1=ctx(id=0,off=0,imm=0) R2_w=inv(id=0,smin_value=-9223372036854775807 (0x8000000000000001),smax_value=9223372032559808513 (0x7fffffff00000001),umin_value=1,umax_value=0xffffffff00000001,var_off=(0x1; 0xffffffff00000000),s32_min_value=1,s32_max_value=1,u32_min_value=1,u32_max_value=1) R3_w=inv4294967298 R10=fp0 9: (5f) r2 &= r3 10: R0_w=inv1337 R1=ctx(id=0,off=0,imm=0) R2_w=inv(id=0,smin_value=0,smax_value=4294967296 (0x100000000),umin_value=0,umax_value=0x100000000,var_off=(0x0; 0x100000000),s32_min_value=0,s32_max_value=0,u32_min_value=0,u32_max_value=0) R3_w=inv4294967298 R10=fp0 [...]
Showing the changes to kernel (1 of 1 file). Show all
Unified · Split
| @@ -7084,11 +7084,10 @@ static void scalar32_min_max_and(struct bpf_reg_state *dst_reg, · 7083 unmodified lines | ||
| 7084 | 7084 | s32 smin_val = src_reg->s32_min_value; |
| 7085 | 7085 | u32 umax_val = src_reg->u32_max_value; |
| 7086 | 7086 | |
| 7087 | − /* Assuming scalar64_min_max_and will be called so its safe | |
| 7088 | − * to skip updating register for known 32-bit case. | |
| 7089 | − */ | |
| 7090 | − if (src_known && dst_known) | |
| 7087 | + if (src_known && dst_known) { | |
| 7088 | + __mark_reg32_known(dst_reg, var32_off.value); | |
| 7091 | 7089 | return; |
| 7090 | + } | |
| 7092 | 7091 | |
| 7093 | 7092 | /* We get our minimum from the var_off, since that's inherently |
| 7094 | 7093 | * bitwise. Our maximum is the minimum of the operands' maxima. |
| @@ -7108,7 +7107,6 @@ static void scalar32_min_max_and(struct bpf_reg_state *dst_reg, · 13 unmodified lines | ||
| 7108 | 7107 | dst_reg->s32_min_value = dst_reg->u32_min_value; |
| 7109 | 7108 | dst_reg->s32_max_value = dst_reg->u32_max_value; |
| 7110 | 7109 | } |
| 7111 | − | |
| 7112 | 7110 | } |
| 7113 | 7111 | |
| 7114 | 7112 | static void scalar_min_max_and(struct bpf_reg_state *dst_reg, |
| @@ -7155,11 +7153,10 @@ static void scalar32_min_max_or(struct bpf_reg_state *dst_reg, · 40 unmodified lines | ||
| 7155 | 7153 | s32 smin_val = src_reg->s32_min_value; |
| 7156 | 7154 | u32 umin_val = src_reg->u32_min_value; |
| 7157 | 7155 | |
| 7158 | − /* Assuming scalar64_min_max_or will be called so it is safe | |
| 7159 | − * to skip updating register for known case. | |
| 7160 | − */ | |
| 7161 | − if (src_known && dst_known) | |
| 7156 | + if (src_known && dst_known) { | |
| 7157 | + __mark_reg32_known(dst_reg, var32_off.value); | |
| 7162 | 7158 | return; |
| 7159 | + } | |
| 7163 | 7160 | |
| 7164 | 7161 | /* We get our maximum from the var_off, and our minimum is the |
| 7165 | 7162 | * maximum of the operands' minima |
| @@ -7224,11 +7221,10 @@ static void scalar32_min_max_xor(struct bpf_reg_state *dst_reg, · 58 unmodified lines | ||
| 7224 | 7221 | struct tnum var32_off = tnum_subreg(dst_reg->var_off); |
| 7225 | 7222 | s32 smin_val = src_reg->s32_min_value; |
| 7226 | 7223 | |
| 7227 | − /* Assuming scalar64_min_max_xor will be called so it is safe | |
| 7228 | − * to skip updating register for known case. | |
| 7229 | − */ | |
| 7230 | − if (src_known && dst_known) | |
| 7224 | + if (src_known && dst_known) { | |
| 7225 | + __mark_reg32_known(dst_reg, var32_off.value); | |
| 7231 | 7226 | return; |
| 7227 | + } | |
| 7232 | 7228 | |
| 7233 | 7229 | /* We get both minimum and maximum from the var32_off. */ |
| 7234 | 7230 | dst_reg->u32_min_value = var32_off.value; |