vboot2: fix alignment issues on 32-bit architectures

We were assuming 8-byte alignment for buffers. That's not true on
32-bit architectures. We should make the alignment requirements
explicit (and correct) for all architectures.

BUG=chromium:452179
BRANCH=ToT
CQ-DEPEND=CL:243380
TEST=manual

  USE=vboot2 FEATURES=test emerge-x86-alex vboot_reference

Change-Id: I120f23e9c5312d7c21ff9ebb6eea2bac1e430e37
Signed-off-by: Bill Richardson <wfrichar@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/243362
Reviewed-by: Randall Spangler <rspangler@chromium.org>

20 files changed, 68 insertions, 35 deletions

diff --git a/firmware/2lib/include/2api.h b/firmware/2lib/include/2api.h
index 0c5792d8..9db5019f 100644
--- a/firmware/2lib/include/2api.h
+++ b/firmware/2lib/include/2api.h
@@ -21,6 +21,7 @@
 #define VBOOT_2_API_H_
 #include <stdint.h>
 
+#include "2common.h"
 #include "2crypto.h"
 #include "2fw_hash_tags.h"
 #include "2guid.h"
diff --git a/firmware/2lib/include/2common.h b/firmware/2lib/include/2common.h
index 9b4a1eb9..69a238c1 100644
--- a/firmware/2lib/include/2common.h
+++ b/firmware/2lib/include/2common.h
@@ -39,8 +39,22 @@ struct vb2_public_key;
 #  define VB2_DEBUG(format, args...)
 #endif
 
-/* Alignment for work buffer pointers/allocations */
-#define VB2_WORKBUF_ALIGN 8
+/*
+ * Alignment for work buffer pointers/allocations should be useful for any
+ * data type. When declaring workbuf buffers on the stack, the caller should
+ * use explicit alignment to avoid run-time errors. For example:
+ *
+ *    int foo(void)
+ *    {
+ *        struct vb2_workbuf wb;
+ *        uint8_t buf[NUM] __attribute__ ((aligned (VB2_WORKBUF_ALIGN)));
+ *        wb.buf = buf;
+ *        wb.size = sizeof(buf);
+ */
+
+/* We might get away with using __alignof__(void *), but since GCC defines a
+ * macro for us we'll be safe and use that. */
+#define VB2_WORKBUF_ALIGN __BIGGEST_ALIGNMENT__
 
 /* Work buffer */
 struct vb2_workbuf {
diff --git a/futility/cmd_vb2_verify_fw.c b/futility/cmd_vb2_verify_fw.c
index 989fc782..53079bb1 100644
--- a/futility/cmd_vb2_verify_fw.c
+++ b/futility/cmd_vb2_verify_fw.c
@@ -140,7 +140,7 @@ static void print_help(const char *progname)
 static int do_vb2_verify_fw(int argc, char *argv[])
 {
 	struct vb2_context ctx;
-	uint8_t workbuf[16384];
+	uint8_t workbuf[16384] __attribute__ ((aligned (VB2_WORKBUF_ALIGN)));
 	int rv;
 
 	if (argc < 4) {
diff --git a/tests/vb20_api_tests.c b/tests/vb20_api_tests.c
index fbde39d2..90141a9b 100644
--- a/tests/vb20_api_tests.c
+++ b/tests/vb20_api_tests.c
@@ -18,7 +18,7 @@
 
 /* Common context for tests */
 static uint8_t workbuf[VB2_WORKBUF_RECOMMENDED_SIZE]
-	__attribute__ ((aligned (16)));
+	__attribute__ ((aligned (VB2_WORKBUF_ALIGN)));
 static struct vb2_context cc;
 static struct vb2_shared_data *sd;
 
diff --git a/tests/vb20_common2_tests.c b/tests/vb20_common2_tests.c
index 75e05249..e5463e28 100644
--- a/tests/vb20_common2_tests.c
+++ b/tests/vb20_common2_tests.c
@@ -77,7 +77,8 @@ static void test_unpack_key(const struct vb2_packed_key *key1)
 static void test_verify_data(const struct vb2_packed_key *key1,
 			     const struct vb2_signature *sig)
 {
-	uint8_t workbuf[VB2_VERIFY_DATA_WORKBUF_BYTES];
+	uint8_t workbuf[VB2_VERIFY_DATA_WORKBUF_BYTES]
+		 __attribute__ ((aligned (VB2_WORKBUF_ALIGN)));
 	struct vb2_workbuf wb;
 
 	uint32_t pubkey_size = key1->key_offset + key1->key_size;
diff --git a/tests/vb20_common3_tests.c b/tests/vb20_common3_tests.c
index 365c7700..9d4cc50b 100644
--- a/tests/vb20_common3_tests.c
+++ b/tests/vb20_common3_tests.c
@@ -33,7 +33,8 @@ static void test_verify_keyblock(const VbPublicKey *public_key,
 				 const VbPrivateKey *private_key,
 				 const VbPublicKey *data_key)
 {
-	uint8_t workbuf[VB2_KEY_BLOCK_VERIFY_WORKBUF_BYTES];
+	uint8_t workbuf[VB2_KEY_BLOCK_VERIFY_WORKBUF_BYTES]
+		__attribute__ ((aligned (VB2_WORKBUF_ALIGN)));
 	struct vb2_workbuf wb;
 	struct vb2_public_key key;
 	struct vb2_keyblock *hdr;
@@ -175,7 +176,8 @@ static void test_verify_fw_preamble(const VbPublicKey *public_key,
 	struct vb2_fw_preamble *hdr;
 	struct vb2_fw_preamble *h;
 	struct vb2_public_key rsa;
-	uint8_t workbuf[VB2_VERIFY_FIRMWARE_PREAMBLE_WORKBUF_BYTES];
+	uint8_t workbuf[VB2_VERIFY_FIRMWARE_PREAMBLE_WORKBUF_BYTES]
+		 __attribute__ ((aligned (VB2_WORKBUF_ALIGN)));
 	struct vb2_workbuf wb;
 	uint32_t hsize;
 
diff --git a/tests/vb20_misc_tests.c b/tests/vb20_misc_tests.c
index b291b4d3..45985dd9 100644
--- a/tests/vb20_misc_tests.c
+++ b/tests/vb20_misc_tests.c
@@ -18,7 +18,7 @@
 
 /* Common context for tests */
 static uint8_t workbuf[VB2_WORKBUF_RECOMMENDED_SIZE]
-	__attribute__ ((aligned (16)));
+	__attribute__ ((aligned (VB2_WORKBUF_ALIGN)));
 static struct vb2_context cc;
 static struct vb2_shared_data *sd;
 
diff --git a/tests/vb20_rsa_padding_tests.c b/tests/vb20_rsa_padding_tests.c
index e9789e90..3bbebcf6 100644
--- a/tests/vb20_rsa_padding_tests.c
+++ b/tests/vb20_rsa_padding_tests.c
@@ -42,7 +42,8 @@ void vb2_public_key_to_vb2(struct vb2_public_key *k2,
  */
 static void test_signatures(const struct vb2_public_key *key)
 {
-	uint8_t workbuf[VB2_VERIFY_DIGEST_WORKBUF_BYTES];
+	uint8_t workbuf[VB2_VERIFY_DIGEST_WORKBUF_BYTES]
+		 __attribute__ ((aligned (VB2_WORKBUF_ALIGN)));
 	uint8_t sig[RSA1024NUMBYTES];
 	struct vb2_workbuf wb;
 	int unexpected_success;
@@ -74,7 +75,8 @@ static void test_signatures(const struct vb2_public_key *key)
  * Test other error conditions in vb2_rsa_verify_digest().
  */
 static void test_verify_digest(struct vb2_public_key *key) {
-	uint8_t workbuf[VB2_VERIFY_DIGEST_WORKBUF_BYTES];
+	uint8_t workbuf[VB2_VERIFY_DIGEST_WORKBUF_BYTES]
+		 __attribute__ ((aligned (VB2_WORKBUF_ALIGN)));
 	uint8_t sig[RSA1024NUMBYTES];
 	struct vb2_workbuf wb;
 	enum vb2_signature_algorithm orig_key_alg = key->sig_alg;
diff --git a/tests/vb21_api_tests.c b/tests/vb21_api_tests.c
index c825bdbf..dbc4750d 100644
--- a/tests/vb21_api_tests.c
+++ b/tests/vb21_api_tests.c
@@ -24,7 +24,7 @@
 
 /* Common context for tests */
 static uint8_t workbuf[VB2_WORKBUF_RECOMMENDED_SIZE]
-	__attribute__ ((aligned (16)));
+	__attribute__ ((aligned (VB2_WORKBUF_ALIGN)));
 static struct vb2_context ctx;
 static struct vb2_shared_data *sd;
 
diff --git a/tests/vb21_common2_tests.c b/tests/vb21_common2_tests.c
index 199a063a..a88e126b 100644
--- a/tests/vb21_common2_tests.c
+++ b/tests/vb21_common2_tests.c
@@ -169,7 +169,8 @@ static void test_verify_signature(const struct vb2_signature *sig)
 static void test_verify_data(const struct vb2_public_key *pubk_orig,
 			      const struct vb2_signature *sig)
 {
-	uint8_t workbuf[VB2_VERIFY_DATA_WORKBUF_BYTES];
+	uint8_t workbuf[VB2_VERIFY_DATA_WORKBUF_BYTES]
+		 __attribute__ ((aligned (VB2_WORKBUF_ALIGN)));
 	struct vb2_workbuf wb;
 
 	struct vb2_public_key pubk;
diff --git a/tests/vb21_common_tests.c b/tests/vb21_common_tests.c
index 74440254..d2ac0edf 100644
--- a/tests/vb21_common_tests.c
+++ b/tests/vb21_common_tests.c
@@ -220,7 +220,8 @@ static void test_verify_hash(void)
 	struct vb2_signature *sig;
 	const struct vb2_private_key *prik;
 	struct vb2_public_key pubk;
-	uint8_t workbuf[VB2_VERIFY_DATA_WORKBUF_BYTES];
+	uint8_t workbuf[VB2_VERIFY_DATA_WORKBUF_BYTES]
+		 __attribute__ ((aligned (VB2_WORKBUF_ALIGN)));
 	struct vb2_workbuf wb;
 
 	vb2_workbuf_init(&wb, workbuf, sizeof(workbuf));
@@ -258,7 +259,8 @@ static void test_verify_keyblock(void)
 	uint32_t buf_size;
 	uint8_t *buf, *buf2;
 
-	uint8_t workbuf[VB2_KEY_BLOCK_VERIFY_WORKBUF_BYTES];
+	uint8_t workbuf[VB2_KEY_BLOCK_VERIFY_WORKBUF_BYTES]
+		 __attribute__ ((aligned (VB2_WORKBUF_ALIGN)));
 	struct vb2_workbuf wb;
 
 	TEST_SUCC(vb2_public_key_hash(&pubk, VB2_HASH_SHA256),
@@ -384,7 +386,8 @@ static void test_verify_fw_preamble(void)
 	uint32_t buf_size;
 	uint8_t *buf, *buf2;
 
-	uint8_t workbuf[VB2_VERIFY_FIRMWARE_PREAMBLE_WORKBUF_BYTES];
+	uint8_t workbuf[VB2_VERIFY_FIRMWARE_PREAMBLE_WORKBUF_BYTES]
+		 __attribute__ ((aligned (VB2_WORKBUF_ALIGN)));
 	struct vb2_workbuf wb;
 
 	/*
diff --git a/tests/vb21_host_fw_preamble_tests.c b/tests/vb21_host_fw_preamble_tests.c
index 8bb83d05..5edcb51d 100644
--- a/tests/vb21_host_fw_preamble_tests.c
+++ b/tests/vb21_host_fw_preamble_tests.c
@@ -39,7 +39,8 @@ static void preamble_tests(const char *keys_dir)
 	uint32_t hash_next;
 	int i;
 
-	uint8_t workbuf[VB2_VERIFY_FIRMWARE_PREAMBLE_WORKBUF_BYTES];
+	uint8_t workbuf[VB2_VERIFY_FIRMWARE_PREAMBLE_WORKBUF_BYTES]
+		 __attribute__ ((aligned (VB2_WORKBUF_ALIGN)));
 	struct vb2_workbuf wb;
 
 	vb2_workbuf_init(&wb, workbuf, sizeof(workbuf));
diff --git a/tests/vb21_host_keyblock_tests.c b/tests/vb21_host_keyblock_tests.c
index 95dbe98e..586dc7b6 100644
--- a/tests/vb21_host_keyblock_tests.c
+++ b/tests/vb21_host_keyblock_tests.c
@@ -29,7 +29,8 @@ static void keyblock_tests(const char *keys_dir)
 	char fname[1024];
 	const char test_desc[] = "Test keyblock";
 
-	uint8_t workbuf[VB2_KEY_BLOCK_VERIFY_WORKBUF_BYTES];
+	uint8_t workbuf[VB2_KEY_BLOCK_VERIFY_WORKBUF_BYTES]
+		 __attribute__ ((aligned (VB2_WORKBUF_ALIGN)));
 	struct vb2_workbuf wb;
 
 	vb2_workbuf_init(&wb, workbuf, sizeof(workbuf));
diff --git a/tests/vb21_host_sig_tests.c b/tests/vb21_host_sig_tests.c
index 66e91553..4a1943ab 100644
--- a/tests/vb21_host_sig_tests.c
+++ b/tests/vb21_host_sig_tests.c
@@ -46,7 +46,8 @@ static void sig_tests(const struct alg_combo *combo,
 	struct vb2_signature *sig, *sig2;
 	uint32_t size;
 
-	uint8_t workbuf[VB2_VERIFY_DATA_WORKBUF_BYTES];
+	uint8_t workbuf[VB2_VERIFY_DATA_WORKBUF_BYTES]
+		 __attribute__ ((aligned (VB2_WORKBUF_ALIGN)));
 	struct vb2_workbuf wb;
 
 	uint8_t *buf;
diff --git a/tests/vb21_misc_tests.c b/tests/vb21_misc_tests.c
index ff3f2766..af2c0792 100644
--- a/tests/vb21_misc_tests.c
+++ b/tests/vb21_misc_tests.c
@@ -20,7 +20,7 @@
 
 /* Common context for tests */
 static uint8_t workbuf[VB2_WORKBUF_RECOMMENDED_SIZE]
-	__attribute__ ((aligned (16)));
+	__attribute__ ((aligned (VB2_WORKBUF_ALIGN)));
 static struct vb2_context ctx;
 static struct vb2_shared_data *sd;
 
diff --git a/tests/vb2_api_tests.c b/tests/vb2_api_tests.c
index ddbd8164..a68b6c56 100644
--- a/tests/vb2_api_tests.c
+++ b/tests/vb2_api_tests.c
@@ -18,7 +18,7 @@
 
 /* Common context for tests */
 static uint8_t workbuf[VB2_WORKBUF_RECOMMENDED_SIZE]
-	__attribute__ ((aligned (16)));
+	__attribute__ ((aligned (VB2_WORKBUF_ALIGN)));
 static struct vb2_context cc;
 static struct vb2_shared_data *sd;
 
diff --git a/tests/vb2_common_tests.c b/tests/vb2_common_tests.c
index 4c033fbd..c7e99769 100644
--- a/tests/vb2_common_tests.c
+++ b/tests/vb2_common_tests.c
@@ -63,28 +63,32 @@ static void test_align(void)
  */
 static void test_workbuf(void)
 {
-	uint64_t buf[8];
+	uint64_t buf[8] __attribute__ ((aligned (VB2_WORKBUF_ALIGN)));
 	uint8_t *p0 = (uint8_t *)buf, *ptr;
 	struct vb2_workbuf wb;
 
+	/* NOTE: There are several magic numbers below which assume that
+	 * VB2_WORKBUF_ALIGN == 16 */
+
 	/* Init */
-	vb2_workbuf_init(&wb, p0, 32);
+	vb2_workbuf_init(&wb, p0, 64);
 	TEST_EQ(vb2_offset_of(p0, wb.buf), 0, "Workbuf init aligned");
-	TEST_EQ(wb.size, 32, "  size");
+	TEST_EQ(wb.size, 64, "  size");
 
-	vb2_workbuf_init(&wb, p0 + 4, 32);
-	TEST_EQ(vb2_offset_of(p0, wb.buf), 8, "Workbuf init unaligned");
-	TEST_EQ(wb.size, 28, "  size");
+	vb2_workbuf_init(&wb, p0 + 4, 64);
+	TEST_EQ(vb2_offset_of(p0, wb.buf), VB2_WORKBUF_ALIGN,
+		"Workbuf init unaligned");
+	TEST_EQ(wb.size, 64 - VB2_WORKBUF_ALIGN + 4, "  size");
 
 	vb2_workbuf_init(&wb, p0 + 2, 5);
 	TEST_EQ(wb.size, 0, "Workbuf init tiny unaligned size");
 
 	/* Alloc rounds up */
-	vb2_workbuf_init(&wb, p0, 32);
+	vb2_workbuf_init(&wb, p0, 64);
 	ptr = vb2_workbuf_alloc(&wb, 22);
 	TEST_EQ(vb2_offset_of(p0, ptr), 0, "Workbuf alloc");
-	TEST_EQ(vb2_offset_of(p0, wb.buf), 24, "  buf");
-	TEST_EQ(wb.size, 8, "  size");
+	TEST_EQ(vb2_offset_of(p0, wb.buf), 32, "  buf");
+	TEST_EQ(wb.size, 32, "  size");
 
 	vb2_workbuf_init(&wb, p0, 32);
 	TEST_PTR_EQ(vb2_workbuf_alloc(&wb, 33), NULL, "Workbuf alloc too big");
@@ -97,12 +101,12 @@ static void test_workbuf(void)
 	TEST_EQ(wb.size, 32, "  size");
 
 	/* Realloc keeps same pointer as alloc */
-	vb2_workbuf_init(&wb, p0, 32);
+	vb2_workbuf_init(&wb, p0, 64);
 	vb2_workbuf_alloc(&wb, 6);
 	ptr = vb2_workbuf_realloc(&wb, 6, 21);
 	TEST_EQ(vb2_offset_of(p0, ptr), 0, "Workbuf realloc");
-	TEST_EQ(vb2_offset_of(p0, wb.buf), 24, "  buf");
-	TEST_EQ(wb.size, 8, "  size");
+	TEST_EQ(vb2_offset_of(p0, wb.buf), 32, "  buf");
+	TEST_EQ(wb.size, 32, "  size");
 }
 
 int main(int argc, char* argv[])
diff --git a/tests/vb2_misc_tests.c b/tests/vb2_misc_tests.c
index 3dcb1dad..437b247f 100644
--- a/tests/vb2_misc_tests.c
+++ b/tests/vb2_misc_tests.c
@@ -16,7 +16,7 @@
 
 /* Common context for tests */
 static uint8_t workbuf[VB2_WORKBUF_RECOMMENDED_SIZE]
-	__attribute__ ((aligned (16)));
+	__attribute__ ((aligned (VB2_WORKBUF_ALIGN)));
 static struct vb2_context cc;
 static struct vb2_shared_data *sd;
 
diff --git a/tests/vb2_nvstorage_tests.c b/tests/vb2_nvstorage_tests.c
index 46547f8f..1471e1bf 100644
--- a/tests/vb2_nvstorage_tests.c
+++ b/tests/vb2_nvstorage_tests.c
@@ -63,7 +63,8 @@ static void nv_storage_test(void)
 {
 	struct nv_field *vnf;
 	uint8_t goodcrc;
-	uint8_t workbuf[VB2_WORKBUF_RECOMMENDED_SIZE];
+	uint8_t workbuf[VB2_WORKBUF_RECOMMENDED_SIZE]
+		__attribute__ ((aligned (VB2_WORKBUF_ALIGN)));
 	struct vb2_context c = {
 		.flags = 0,
 		.workbuf = workbuf,
diff --git a/tests/vb2_secdata_tests.c b/tests/vb2_secdata_tests.c
index 51283317..d4344b0b 100644
--- a/tests/vb2_secdata_tests.c
+++ b/tests/vb2_secdata_tests.c
@@ -29,7 +29,8 @@ static void test_changed(struct vb2_context *ctx, int changed, const char *why)
 
 static void secdata_test(void)
 {
-	uint8_t workbuf[VB2_WORKBUF_RECOMMENDED_SIZE];
+	uint8_t workbuf[VB2_WORKBUF_RECOMMENDED_SIZE]
+		__attribute__ ((aligned (VB2_WORKBUF_ALIGN)));
 	struct vb2_context c = {
 		.flags = 0,
 		.workbuf = workbuf,