MUL instruction required that src_reg would be a known value (i.e.
src_reg would be evaluate as a const value). The condition in this case
can be relaxed, since multiplication is a commutative operator and the
range computation is still valid if at least one of its registers is
known.
BPF self-tests were added to check the new functionality.
Signed-off-by: Cupertino Miranda <cupertino.miranda@xxxxxxxxxx>
---
kernel/bpf/verifier.c | 10 +-
.../selftests/bpf/progs/verifier_bounds.c | 99 +++++++++++++++++++
2 files changed, 105 insertions(+), 4 deletions(-)
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 7894af2e1bdb..a326ec024d82 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -13741,15 +13741,17 @@ static bool is_const_reg_and_valid(struct bpf_reg_state reg, bool alu32,
}
static bool is_safe_to_compute_dst_reg_ranges(struct bpf_insn *insn,
+ struct bpf_reg_state dst_reg,
struct bpf_reg_state src_reg)
{
- bool src_known;
+ bool src_known, dst_known;
u64 insn_bitness = (BPF_CLASS(insn->code) == BPF_ALU64) ? 64 : 32;
bool alu32 = (BPF_CLASS(insn->code) != BPF_ALU64);
u8 opcode = BPF_OP(insn->code);
bool valid_known = true;
src_known = is_const_reg_and_valid(src_reg, alu32, &valid_known);
+ dst_known = is_const_reg_and_valid(dst_reg, alu32, &valid_known);
/* Taint dst register if offset had invalid bounds
* derived from e.g. dead branches.
@@ -13765,10 +13767,10 @@ static bool is_safe_to_compute_dst_reg_ranges(struct bpf_insn *insn,
case BPF_OR:
return true;
- /* Compute range for MUL if the src_reg is known.
+ /* Compute range for MUL if at least one of its registers is know.
*/
case BPF_MUL:
- return src_known;
+ return src_known || dst_known;
/* Shift operators range is only computable if shift dimension operand
* is known. Also, shifts greater than 31 or 63 are undefined. This
@@ -13799,7 +13801,7 @@ static int adjust_scalar_min_max_vals(struct bpf_verifier_env *env,
bool alu32 = (BPF_CLASS(insn->code) != BPF_ALU64);
int ret;
- if (!is_safe_to_compute_dst_reg_ranges(insn, src_reg)) {
+ if (!is_safe_to_compute_dst_reg_ranges(insn, *dst_reg, src_reg)) {
__mark_reg_unknown(env, dst_reg);
return 0;
}
diff --git a/tools/testing/selftests/bpf/progs/verifier_bounds.c b/tools/testing/selftests/bpf/progs/verifier_bounds.c
index 2fcf46341b30..09bb1b270ca7 100644
--- a/tools/testing/selftests/bpf/progs/verifier_bounds.c
+++ b/tools/testing/selftests/bpf/progs/verifier_bounds.c
@@ -949,6 +949,105 @@ l1_%=: r0 = 0; \
: __clobber_all);
}
+SEC("socket")
+__description("bounds check for reg32 <= 9, 3 mul (0,3)")
+__success __failure_unpriv
+__msg_unpriv("R0 min value is outside of the allowed memory range")
+__retval(0)
+__naked void reg32_3_mul_reg_01(void)
+{
+ asm volatile (" \
+ call %[bpf_get_prandom_u32]; \
+ r6 = r0; \
+ r1 = 0; \
+ *(u64*)(r10 - 8) = r1; \
+ r2 = r10; \
+ r2 += -8; \
+ r1 = %[map_hash_8b] ll; \
+ call %[bpf_map_lookup_elem]; \
+ if r0 != 0 goto l0_%=; \
+ exit; \
+l0_%=: w1 = 3; \
+ r6 >>= 62; \
+ w1 *= w6; \
+ if w1 <= 9 goto l1_%=; \
+ r0 = *(u64*)(r0 + 8); \
+l1_%=: r0 = 0; \
+ exit; \
+" :
+ : __imm(bpf_map_lookup_elem),
+ __imm_addr(map_hash_8b),
+ __imm(bpf_get_prandom_u32)
+ : __clobber_all);
+}
+
+SEC("socket")
+__description("bounds check for reg32 <= 9, (0,3) mul 3")
+__success __failure_unpriv
+__msg_unpriv("R0 min value is outside of the allowed memory range")
+__retval(0)
+__naked void reg32_13_mul_reg_3(void)
+{
+ asm volatile (" \
+ call %[bpf_get_prandom_u32]; \
+ r6 = r0; \
+ r1 = 0; \
+ *(u64*)(r10 - 8) = r1; \
+ r2 = r10; \
+ r2 += -8; \
+ r1 = %[map_hash_8b] ll; \
+ call %[bpf_map_lookup_elem]; \
+ if r0 != 0 goto l0_%=; \
+ exit; \
+l0_%=: w1 = 3; \
+ r6 >>= 62; \
+ w6 *= w1; \
+ if w6 <= 9 goto l1_%=; \
+ r0 = *(u64*)(r0 + 8); \
+l1_%=: r0 = 0; \
+ exit; \
+" :
+ : __imm(bpf_map_lookup_elem),
+ __imm_addr(map_hash_8b),
+ __imm(bpf_get_prandom_u32)
+ : __clobber_all);
+}
+
+SEC("socket")
+__description("bounds check for reg32 >= 6 && reg32 <= 15, (2,5) mul 3")
+__success __failure_unpriv
+__msg_unpriv("R0 min value is outside of the allowed memory range")
+__retval(0)
+__naked void reg32_25_mul_reg_3(void)
+{
+ asm volatile (" \
+ call %[bpf_get_prandom_u32]; \
+ r6 = r0; \
+ r1 = 0; \
+ *(u64*)(r10 - 8) = r1; \
+ r2 = r10; \
+ r2 += -8; \
+ r1 = %[map_hash_8b] ll; \
+ call %[bpf_map_lookup_elem]; \
+ if r0 != 0 goto l0_%=; \
+ exit; \
+l0_%=: w1 = 3; \
+ r6 >>= 62; \
+ r6 += 2; \
+ w6 *= w1; \
+ if w6 > 15 goto l1_%=; \
+ if w6 < 6 goto l1_%=; \
+ r0 = 0; \
+ exit; \
+l1_%=: r0 = *(u64*)(r0 + 8); \
+ exit; \
+" :
+ : __imm(bpf_map_lookup_elem),
+ __imm_addr(map_hash_8b),
+ __imm(bpf_get_prandom_u32)
+ : __clobber_all);
+}
+
SEC("socket")
__description("bounds checks after 32-bit truncation. test 1")
__success __failure_unpriv __msg_unpriv("R0 leaks addr")
--
2.30.2
