Re: [PATCH bpf-next 1/2] bpf: avoid verifier failure for 32bit pointer arithmetic

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

 





On 6/19/20 2:49 PM, Daniel Borkmann wrote:
On 6/19/20 1:46 AM, Yonghong Song wrote:
When do experiments with llvm (disabling instcombine and
simplifyCFG), I hit the following error with test_seg6_loop.o.

   ; R1=pkt(id=0,off=0,r=48,imm=0), R7=pkt(id=0,off=40,r=48,imm=0)
   w2 = w7
   ; R2_w=inv(id=0,umax_value=4294967295,var_off=(0x0; 0xffffffff))
   w2 -= w1
   R2 32-bit pointer arithmetic prohibited

The corresponding source code is:
   uint32_t srh_off
   // srh and skb->data are all packet pointers
   srh_off = (char *)srh - (char *)(long)skb->data;

The verifier does not support 32-bit pointer/scalar arithmetic.

Without my llvm change, the code looks like

   ; R3=pkt(id=0,off=40,r=48,imm=0), R8=pkt(id=0,off=0,r=48,imm=0)
   w3 -= w8
   ; R3_w=inv(id=0)

This is explicitly allowed in verifier if both registers are
pointers and the opcode is BPF_SUB.

To fix this problem, I changed the verifier to allow
32-bit pointer/scaler BPF_SUB operations.

At the source level, the issue could be workarounded with
inline asm or changing "uint32_t srh_off" to "uint64_t srh_off".
But I feel that verifier change might be the right thing to do.

Signed-off-by: Yonghong Song <yhs@xxxxxx>

Looks good, both applied, thanks!

---
  kernel/bpf/verifier.c | 5 +++++
  1 file changed, 5 insertions(+)

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 22d90d47befa..bbf6d655d6ad 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -5052,6 +5052,11 @@ static int adjust_ptr_min_max_vals(struct bpf_verifier_env *env,
      if (BPF_CLASS(insn->code) != BPF_ALU64) {
          /* 32-bit ALU ops on pointers produce (meaningless) scalars */
+        if (opcode == BPF_SUB && env->allow_ptr_leaks) {
+            __mark_reg_unknown(env, dst_reg);
+            return 0;
+        }
+

We could have also allowed ADD case while at it, but ok either way. Wrt pointer sanitation, nothing additional seems to be needed here as we 'destroy' the reg
to fully unknown.

Right. This may happen for use case like `(w1 + off) - w2` where w1 and w2 are packet pointers and off is a register to encode a scalar. I will do a followup on this.


          verbose(env,
              "R%d 32-bit pointer arithmetic prohibited\n",
              dst);





[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