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>
diff --git a/firmware/2lib/include/2api.h b/firmware/2lib/include/2api.h
index 0c5792d..9db5019 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 9b4a1eb..69a238c 100644
--- a/firmware/2lib/include/2common.h
+++ b/firmware/2lib/include/2common.h
@@ -39,8 +39,22 @@
# 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 989fc78..53079bb 100644
--- a/futility/cmd_vb2_verify_fw.c
+++ b/futility/cmd_vb2_verify_fw.c
@@ -140,7 +140,7 @@
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 fbde39d..90141a9 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 75e0524..e5463e2 100644
--- a/tests/vb20_common2_tests.c
+++ b/tests/vb20_common2_tests.c
@@ -77,7 +77,8 @@
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 365c770..9d4cc50 100644
--- a/tests/vb20_common3_tests.c
+++ b/tests/vb20_common3_tests.c
@@ -33,7 +33,8 @@
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 @@
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 b291b4d..45985dd 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 e9789e9..3bbebcf 100644
--- a/tests/vb20_rsa_padding_tests.c
+++ b/tests/vb20_rsa_padding_tests.c
@@ -42,7 +42,8 @@
*/
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 @@
* 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 c825bdb..dbc4750 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 199a063..a88e126 100644
--- a/tests/vb21_common2_tests.c
+++ b/tests/vb21_common2_tests.c
@@ -169,7 +169,8 @@
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 7444025..d2ac0ed 100644
--- a/tests/vb21_common_tests.c
+++ b/tests/vb21_common_tests.c
@@ -220,7 +220,8 @@
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 @@
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 @@
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 8bb83d0..5edcb51 100644
--- a/tests/vb21_host_fw_preamble_tests.c
+++ b/tests/vb21_host_fw_preamble_tests.c
@@ -39,7 +39,8 @@
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 95dbe98..586dc7b 100644
--- a/tests/vb21_host_keyblock_tests.c
+++ b/tests/vb21_host_keyblock_tests.c
@@ -29,7 +29,8 @@
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 66e9155..4a1943a 100644
--- a/tests/vb21_host_sig_tests.c
+++ b/tests/vb21_host_sig_tests.c
@@ -46,7 +46,8 @@
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 ff3f276..af2c079 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 ddbd816..a68b6c5 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 4c033fb..c7e9976 100644
--- a/tests/vb2_common_tests.c
+++ b/tests/vb2_common_tests.c
@@ -63,28 +63,32 @@
*/
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;
- /* Init */
- vb2_workbuf_init(&wb, p0, 32);
- TEST_EQ(vb2_offset_of(p0, wb.buf), 0, "Workbuf init aligned");
- TEST_EQ(wb.size, 32, " size");
+ /* NOTE: There are several magic numbers below which assume that
+ * VB2_WORKBUF_ALIGN == 16 */
- 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");
+ /* Init */
+ vb2_workbuf_init(&wb, p0, 64);
+ TEST_EQ(vb2_offset_of(p0, wb.buf), 0, "Workbuf init aligned");
+ TEST_EQ(wb.size, 64, " 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 @@
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 3dcb1da..437b247 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 46547f8..1471e1b 100644
--- a/tests/vb2_nvstorage_tests.c
+++ b/tests/vb2_nvstorage_tests.c
@@ -63,7 +63,8 @@
{
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 5128331..d4344b0 100644
--- a/tests/vb2_secdata_tests.c
+++ b/tests/vb2_secdata_tests.c
@@ -29,7 +29,8 @@
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,