vboot: use vb2_safe_memcmp instead of SafeMemcmp

No need to have two implementations of this now.

BUG=chromium:611535
BRANCH=none
TEST=make runtests; emerge-kevin coreboot depthcharge

Change-Id: I18bac928eb09971c37f3e1d7cbfd2009999b1f31
Signed-off-by: Randall Spangler <rspangler@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/400899
Reviewed-by: Daisuke Nojiri <dnojiri@chromium.org>

8 files changed, 54 insertions, 105 deletions

diff --git a/Makefile b/Makefile
index 53bc4f8e..f2969d1d 100644
--- a/Makefile
+++ b/Makefile
@@ -317,7 +317,6 @@ BDBLIB = ${BUILD}/bdb.a
 
 # Firmware library sources needed by VbInit() call
 VBINIT_SRCS = \
-	firmware/lib/utility.c \
 	firmware/lib/vboot_common_init.c \
 	firmware/lib/vboot_nvstorage.c \
 	firmware/lib/region-init.c \
diff --git a/firmware/lib/cryptolib/rsa.c b/firmware/lib/cryptolib/rsa.c
index db60d2d2..a4c7262b 100644
--- a/firmware/lib/cryptolib/rsa.c
+++ b/firmware/lib/cryptolib/rsa.c
@@ -10,6 +10,8 @@
 
 #include "sysincludes.h"
 
+#include "2sysincludes.h"
+#include "2common.h"
 #include "cryptolib.h"
 #include "vboot_api.h"
 #include "utility.h"
@@ -167,16 +169,16 @@ int RSAVerify(const RSAPublicKey *key,
   padding_len = padding_size_map[sig_type];
 
   /* Even though there are probably no timing issues here, we use
-   * SafeMemcmp() just to be on the safe side. */
+   * vb2_safe_memcmp() just to be on the safe side. */
 
   /* Check pkcs1.5 padding bytes. */
-  if (SafeMemcmp(buf, padding, padding_len)) {
+  if (vb2_safe_memcmp(buf, padding, padding_len)) {
     VBDEBUG(("In RSAVerify(): Padding check failed!\n"));
     success = 0;
   }
 
   /* Check hash. */
-  if (SafeMemcmp(buf + padding_len, hash, sig_len - padding_len)) {
+  if (vb2_safe_memcmp(buf + padding_len, hash, sig_len - padding_len)) {
     VBDEBUG(("In RSAVerify(): Hash check failed!\n"));
     success  = 0;
   }
diff --git a/firmware/lib/include/utility.h b/firmware/lib/include/utility.h
index aaddb8c8..9f191701 100644
--- a/firmware/lib/include/utility.h
+++ b/firmware/lib/include/utility.h
@@ -46,18 +46,6 @@
 /* Return the minimum of (a) or (b). */
 #define Min(a, b) (((a) < (b)) ? (a) : (b))
 
-/**
- * Compare [n] bytes starting at [s1] with [s2] and return 0 if they
- * match, 1 if they don't.  Returns 0 if n=0, since no bytes mismatched.
- *
- * Time taken to perform the comparison is only dependent on [n] and
- * not on the relationship of the match between [s1] and [s2].
- *
- * Note that unlike memcmp(), this only indicates inequality, not
- * whether s1 is less than or greater than s2.
- */
-int SafeMemcmp(const void *s1, const void *s2, size_t n);
-
 /*
  * Buffer size required to hold the longest possible output of Uint64ToString()
  * - that is, Uint64ToString(~0, 2).
diff --git a/firmware/lib/utility.c b/firmware/lib/utility.c
deleted file mode 100644
index 2f8c90dc..00000000
--- a/firmware/lib/utility.c
+++ /dev/null
@@ -1,28 +0,0 @@
-/* Copyright (c) 2013 The Chromium OS Authors. All rights reserved.
- * Use of this source code is governed by a BSD-style license that can be
- * found in the LICENSE file.
- *
- * Utility functions that need to be built as part of the firmware.
- */
-
-#include "sysincludes.h"
-
-#include "utility.h"
-
-int SafeMemcmp(const void *s1, const void *s2, size_t n) {
-	const unsigned char *us1 = s1;
-	const unsigned char *us2 = s2;
-	int result = 0;
-
-	if (0 == n)
-		return 0;
-
-	/*
-	 * Code snippet without data-dependent branch due to Nate Lawson
-	 * (nate@root.org) of Root Labs.
-	 */
-	while (n--)
-		result |= *us1++ ^ *us2++;
-
-	return result != 0;
-}
diff --git a/firmware/lib/vboot_api_kernel.c b/firmware/lib/vboot_api_kernel.c
index 6a2aba32..1360971f 100644
--- a/firmware/lib/vboot_api_kernel.c
+++ b/firmware/lib/vboot_api_kernel.c
@@ -7,6 +7,8 @@
 
 #include "sysincludes.h"
 
+#include "2sysincludes.h"
+#include "2common.h"
 #include "gbb_access.h"
 #include "gbb_header.h"
 #include "load_kernel_fw.h"
@@ -759,7 +761,7 @@ static VbError_t EcUpdateImage(int devidx, VbCommonParams *cparams,
 	for (i = 0; i < hash_size; i++)
 		VBDEBUG(("%02x", hash[i]));
 	VBDEBUG(("\n"));
-	*need_update = SafeMemcmp(ec_hash, hash, hash_size);
+	*need_update = vb2_safe_memcmp(ec_hash, hash, hash_size);
 
 	if (!*need_update)
 		return VBERROR_SUCCESS;
@@ -854,7 +856,7 @@ static VbError_t EcUpdateImage(int devidx, VbCommonParams *cparams,
 		VBDEBUG(("%02x",ec_hash[i]));
 	VBDEBUG(("\n"));
 
-	if (SafeMemcmp(ec_hash, hash, hash_size)){
+	if (vb2_safe_memcmp(ec_hash, hash, hash_size)){
 		VBDEBUG(("EcUpdateImage() - "
 			 "Failed to update EC-%s\n", rw_request ?
 			 "RW" : "RO"));
diff --git a/firmware/lib/vboot_common.c b/firmware/lib/vboot_common.c
index b691fc36..56ba35df 100644
--- a/firmware/lib/vboot_common.c
+++ b/firmware/lib/vboot_common.c
@@ -197,7 +197,8 @@ int KeyBlockVerify(const VbKeyBlockHeader *block, uint64_t size,
 		VBDEBUG(("Not enough space for key block header.\n"));
 		return VBOOT_KEY_BLOCK_INVALID;
 	}
-	if (SafeMemcmp(block->magic, KEY_BLOCK_MAGIC, KEY_BLOCK_MAGIC_SIZE)) {
+	if (vb2_safe_memcmp(block->magic, KEY_BLOCK_MAGIC,
+			    KEY_BLOCK_MAGIC_SIZE)) {
 		VBDEBUG(("Not a valid verified boot key block.\n"));
 		return VBOOT_KEY_BLOCK_INVALID;
 	}
@@ -249,8 +250,9 @@ int KeyBlockVerify(const VbKeyBlockHeader *block, uint64_t size,
 				       header_checksum,
 				       sizeof(header_checksum));
 		if (!rv)
-			rv = SafeMemcmp(header_checksum, GetSignatureDataC(sig),
-					sizeof(header_checksum));
+			rv = vb2_safe_memcmp(header_checksum,
+					     GetSignatureDataC(sig),
+					     sizeof(header_checksum));
 
 		if (rv) {
 			VBDEBUG(("Invalid key block hash.\n"));
diff --git a/firmware/lib/vboot_kernel.c b/firmware/lib/vboot_kernel.c
index 1e7b7d0d..be5389a4 100644
--- a/firmware/lib/vboot_kernel.c
+++ b/firmware/lib/vboot_kernel.c
@@ -288,15 +288,17 @@ VbError_t LoadKernel(LoadKernelParams *params, VbCommonParams *cparams)
 			VBDEBUG(("Checking developer key hash.\n"));
 			vb2_digest_buffer(buf, buflen, VB2_HASH_SHA256,
 					  digest, sizeof(digest));
-			if (0 != SafeMemcmp(digest, params->fwmp->dev_key_hash,
-					    VB2_SHA256_DIGEST_SIZE)) {
+			if (0 != vb2_safe_memcmp(digest,
+						 params->fwmp->dev_key_hash,
+						 VB2_SHA256_DIGEST_SIZE)) {
 				int i;
 
 				VBDEBUG(("Wrong developer key hash.\n"));
 				VBDEBUG(("Want: "));
 				for (i = 0; i < VB2_SHA256_DIGEST_SIZE; i++)
 					VBDEBUG(("%02x",
-						 params->fwmp->dev_key_hash[i]));
+						 params->
+						 fwmp->dev_key_hash[i]));
 				VBDEBUG(("\nGot:  "));
 				for (i = 0; i < VB2_SHA256_DIGEST_SIZE; i++)
 					VBDEBUG(("%02x", digest[i]));
diff --git a/tests/utility_tests.c b/tests/utility_tests.c
index 978f768c..88fe1194 100644
--- a/tests/utility_tests.c
+++ b/tests/utility_tests.c
@@ -10,67 +10,49 @@
 #include <stdlib.h>
 #include <string.h>
 
-#define _STUB_IMPLEMENTATION_  /* So we can use memset() ourselves */
-
 #include "test_common.h"
 #include "utility.h"
 #include "vboot_common.h"
 
-
 /* Test utility.h and sysincludes.h macros */
-static void MacrosTest(void) {
-  int64_t a = -10, b = -20;
-  uint64_t u = (0xABCD00000000ULL);
-  uint64_t v = (0xABCD000000ULL);
-
-  TEST_EQ(CombineUint16Pair(1, 2), 0x00010002, "CombineUint16Pair");
-  TEST_EQ(CombineUint16Pair(0xFFFE, 0xFFFF), 0xFFFEFFFF,
-          "CombineUint16Pair 2");
-  TEST_EQ(CombineUint16Pair(-4, -16), 0xFFFCFFF0,
-          "CombineUint16Pair big negative");
-  TEST_EQ(CombineUint16Pair(0x10003, 0x10004), 0x00030004,
-          "CombineUint16Pair overflow");
-
-  TEST_EQ(Min(1, 2), 1, "Min 1");
-  TEST_EQ(Min(4, 3), 3, "Min 2");
-  TEST_EQ(Min(5, 5), 5, "Min 5");
-  TEST_EQ(Min(a, b), b, "Min uint64 1");
-  TEST_EQ(Min(b, a), b, "Min uint64 2");
-  TEST_EQ(Min(b, b), b, "Min uint64 same");
-
-  TEST_EQ(u >> 8, v, "uint64_t >> 8");
-  TEST_EQ(u >> 0, u, "uint64_t >> 0");
-  TEST_EQ(u >> 36, (uint64_t)0xABC, "uint64_t >> 36");
-
-  TEST_EQ(v * (uint32_t)0, 0, "uint64_t * uint32_t 0");
-  TEST_EQ(v * (uint32_t)1, v, "uint64_t * uint32_t 1");
-  TEST_EQ(v * (uint32_t)256, u, "uint64_t * uint32_t 256");
+static void MacrosTest(void)
+{
+	int64_t a = -10, b = -20;
+	uint64_t u = (0xABCD00000000ULL);
+	uint64_t v = (0xABCD000000ULL);
+
+	TEST_EQ(CombineUint16Pair(1, 2), 0x00010002, "CombineUint16Pair");
+	TEST_EQ(CombineUint16Pair(0xFFFE, 0xFFFF), 0xFFFEFFFF,
+		"CombineUint16Pair 2");
+	TEST_EQ(CombineUint16Pair(-4, -16), 0xFFFCFFF0,
+		"CombineUint16Pair big negative");
+	TEST_EQ(CombineUint16Pair(0x10003, 0x10004), 0x00030004,
+		"CombineUint16Pair overflow");
+
+	TEST_EQ(Min(1, 2), 1, "Min 1");
+	TEST_EQ(Min(4, 3), 3, "Min 2");
+	TEST_EQ(Min(5, 5), 5, "Min 5");
+	TEST_EQ(Min(a, b), b, "Min uint64 1");
+	TEST_EQ(Min(b, a), b, "Min uint64 2");
+	TEST_EQ(Min(b, b), b, "Min uint64 same");
+
+	TEST_EQ(u >> 8, v, "uint64_t >> 8");
+	TEST_EQ(u >> 0, u, "uint64_t >> 0");
+	TEST_EQ(u >> 36, (uint64_t)0xABC, "uint64_t >> 36");
+
+	TEST_EQ(v * (uint32_t)0, 0, "uint64_t * uint32_t 0");
+	TEST_EQ(v * (uint32_t)1, v, "uint64_t * uint32_t 1");
+	TEST_EQ(v * (uint32_t)256, u, "uint64_t * uint32_t 256");
 }
 
+int main(int argc, char* argv[])
+{
+	int error_code = 0;
 
-/* Test SafeMemcmp */
-static void SafeMemcmpTest(void) {
-  /* Zero-length strings are equal */
-  TEST_EQ(0, SafeMemcmp("APPLE", "TIGER", 0), "SafeMemcmp() size=0");
-
-  /* Test equal arrays */
-  TEST_EQ(0, SafeMemcmp("clonebob", "clonebob", 8), "SafeMemcmp() equal");
-  /* Inequality past end of array doesn't affect result */
-  TEST_EQ(0, SafeMemcmp("clonebob", "clonedan", 5), "SafeMemcmp() equal2");
-
-  TEST_EQ(1, SafeMemcmp("APPLE", "TIGER", 5), "SafeMemcmp() unequal");
-  TEST_EQ(1, SafeMemcmp("APPLE", "APPLe", 5), "SafeMemcmp() unequal 2");
-}
-
-
-int main(int argc, char* argv[]) {
-  int error_code = 0;
-
-  MacrosTest();
-  SafeMemcmpTest();
+	MacrosTest();
 
-  if (!gTestSuccess)
-    error_code = 255;
+	if (!gTestSuccess)
+		error_code = 255;
 
-  return error_code;
+	return error_code;
 }