Re: [PATCH bpf-next 3/3] bpf: relax MUL range computation check

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 4/11/24 10:37 AM, Cupertino Miranda wrote:
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);

Is it a possible the above could falsely reject some operation since
in the original code, dst_reg is not checked here?

/* 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.

know => known

  	 */
  	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

Let us break this commit into two patches: verifier change and selftest change. This will make possible backport easier.

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);
  }
[...]




[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux