netdev - [RFC PATCH 13/16] bpf: verifier, can ptr range be calculated with scalar ALU op

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20180601093324.15353.14482.stgit@john-Precision-Tower-5810>
Date:   Fri, 01 Jun 2018 02:33:24 -0700
From:   John Fastabend <john.fastabend@...il.com>
To:     alexei.starovoitov@...il.com, daniel@...earbox.net,
        davem@...emloft.net
Cc:     netdev@...r.kernel.org
Subject: [RFC PATCH 13/16] bpf: verifier,
 can ptr range be calculated with scalar ALU op

At the moment any BPF_ADD or BPF_NEG with a pointer type will create
a new pointer and destroy the register range. Then any memory access
after this will fail because it looks like no bounds checking has
been done, even if it was previously done on the other pointer. So
patterns like this fail,

   ptrA = pkt_data;

   if (ptrA + expected_payload > data_end)
          return 0;

   ptrA  += 1
   accum += *ptrA
   ptrA  += 1
   accum += *ptrA

I did a quick look over the code and it seems like if the ALU op
has a uma_val and it is less than the previous range, we can simply
reduce the range by that amount and pull it into the new ptr.

This patch does the above.
---
 kernel/bpf/verifier.c |    6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 610559a..c41f587 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -2830,8 +2830,10 @@ static int adjust_ptr_min_max_vals(struct bpf_verifier_env *env,
 		dst_reg->off = ptr_reg->off;
 		if (reg_is_pkt_pointer(ptr_reg)) {
 			dst_reg->id = ++env->id_gen;
-			/* something was added to pkt_ptr, set range to zero */
-			dst_reg->range = 0;
+			if (umax_val > dst_reg->range)
+				dst_reg->range = 0;
+			else
+				dst_reg->range -= umax_val;
 		}
 		break;
 	case BPF_SUB: