netdev - [PATCH bpf-next] bpf/verifier: properly clear union members after a ctx read

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-next>] [day] [month] [year] [list]

Message-ID: <0b724ba4-9ddf-e635-0bd5-201658355cf5@solarflare.com>
Date:   Tue, 4 Sep 2018 15:19:52 +0100
From:   Edward Cree <ecree@...arflare.com>
To:     <daniel@...earbox.net>, <ast@...nel.org>
CC:     <netdev@...r.kernel.org>
Subject: [PATCH bpf-next] bpf/verifier: properly clear union members after a
 ctx read

In check_mem_access(), for the PTR_TO_CTX case, after check_ctx_access()
 has supplied a reg_type, the other members of the register state are set
 appropriately.  Previously reg.range was set to 0, but as it is in a
 union with reg.map_ptr, which is larger, upper bytes of the latter were
 left in place.  This then caused the memcmp() in regsafe() to fail,
 preventing some branches from being pruned (and occasionally causing the
 same program to take a varying number of processed insns on repeated
 verifier runs).

Signed-off-by: Edward Cree <ecree@...arflare.com>
---
Possibly something might need adding to __mark_reg_unknown() as well to
 clear map_ptr/range, I'm not sure (though doing so did not affect the
 processed insn count on the cilium programs).

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

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index f4ff0c569e54..49e4ea66fdd3 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -1640,9 +1640,9 @@ static int check_mem_access(struct bpf_verifier_env *env, int insn_idx, u32 regn
 			else
 				mark_reg_known_zero(env, regs,
 						    value_regno);
-			regs[value_regno].id = 0;
-			regs[value_regno].off = 0;
-			regs[value_regno].range = 0;
+			/* Clear id, off, and union(map_ptr, range) */
+			memset(regs + value_regno, 0,
+			       offsetof(struct bpf_reg_state, var_off));
 			regs[value_regno].type = reg_type;
 		}