diff options
author | Randall Spangler <rspangler@chromium.org> | 2014-06-06 09:30:14 -0700 |
---|---|---|
committer | chrome-internal-fetch <chrome-internal-fetch@google.com> | 2014-06-11 22:08:10 +0000 |
commit | 2145721c3c1840561030b27d2207006b0139c16c (patch) | |
tree | 8644c3e990a50243e91a7c2dbb6479b10033d67f | |
parent | b9be53640efdee92b1b42e60adda274563236301 (diff) | |
download | vboot-2145721c3c1840561030b27d2207006b0139c16c.tar.gz |
vboot2: Use more specific error codes, part 2
Error codes reported by the aligment checks in common.c are now very
specific, and tests verify the proper errors are reported.
Changed args to vb2_member_inside() so I can force wraparounds.
BUG=chromium:370082
BRANCH=none
TEST=make clean && VBOOT2=1 COV=1 make
Change-Id: Ib135674e82005b76bce7a83a1f4a65a9c5296cf4
Signed-off-by: Randall Spangler <rspangler@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/202937
Reviewed-by: Daisuke Nojiri <dnojiri@chromium.org>
-rw-r--r-- | firmware/2lib/2common.c | 27 | ||||
-rw-r--r-- | firmware/2lib/include/2common.h | 8 | ||||
-rw-r--r-- | firmware/2lib/include/2return_codes.h | 26 | ||||
-rw-r--r-- | tests/vb2_common_tests.c | 107 |
4 files changed, 103 insertions, 65 deletions
diff --git a/firmware/2lib/2common.c b/firmware/2lib/2common.c index 9099f6f7..56593aae 100644 --- a/firmware/2lib/2common.c +++ b/firmware/2lib/2common.c @@ -20,14 +20,14 @@ int vb2_align(uint8_t **ptr, uint32_t *size, uint32_t align, uint32_t want_size) offs = align - offs; if (*size < offs) - return VB2_ERROR_BUFFER_TOO_SMALL; + return VB2_ERROR_ALIGN_BIGGER_THAN_SIZE; *ptr += offs; *size -= offs; } if (*size < want_size) - return VB2_ERROR_BUFFER_TOO_SMALL; + return VB2_ERROR_ALIGN_SIZE; return VB2_SUCCESS; } @@ -106,10 +106,10 @@ ptrdiff_t vb2_offset_of(const void *base, const void *ptr) return (uintptr_t)ptr - (uintptr_t)base; } -int vb2_verify_member_inside(const void *parent, uint32_t parent_size, - const void *member, uint32_t member_size, - uint32_t member_data_offset, - uint32_t member_data_size) +int vb2_verify_member_inside(const void *parent, size_t parent_size, + const void *member, size_t member_size, + ptrdiff_t member_data_offset, + size_t member_data_size) { const size_t psize = (size_t)parent_size; const uintptr_t parent_end = (uintptr_t)parent + parent_size; @@ -120,23 +120,22 @@ int vb2_verify_member_inside(const void *parent, uint32_t parent_size, /* Make sure parent doesn't wrap */ if (parent_end < (uintptr_t)parent) - return VB2_ERROR_UNKNOWN; + return VB2_ERROR_INSIDE_PARENT_WRAPS; /* * Make sure the member is fully contained in the parent and doesn't * wrap. Use >, not >=, since member_size = 0 is possible. */ if (member_end_offs < member_offs) - return VB2_ERROR_UNKNOWN; - if (member_offs > psize || member_end_offs > psize) - return VB2_ERROR_UNKNOWN; - + return VB2_ERROR_INSIDE_MEMBER_WRAPS; + if (member_offs < 0 || member_offs > psize || member_end_offs > psize) + return VB2_ERROR_INSIDE_MEMBER_OUTSIDE; /* Make sure parent fully contains member data */ if (data_end_offs < data_offs) - return VB2_ERROR_UNKNOWN; - if (data_offs > psize || data_end_offs > psize) - return VB2_ERROR_UNKNOWN; + return VB2_ERROR_INSIDE_DATA_WRAPS; + if (data_offs < 0 || data_offs > psize || data_end_offs > psize) + return VB2_ERROR_INSIDE_DATA_OUTSIDE; return VB2_SUCCESS; } diff --git a/firmware/2lib/include/2common.h b/firmware/2lib/include/2common.h index 3bb403be..326f8919 100644 --- a/firmware/2lib/include/2common.h +++ b/firmware/2lib/include/2common.h @@ -151,10 +151,10 @@ uint8_t *vb2_signature_data(struct vb2_signature *sig); * @param member_data_size Size of member data in bytes * @return VB2_SUCCESS, or non-zero if error. */ -int vb2_verify_member_inside(const void *parent, uint32_t parent_size, - const void *member, uint32_t member_size, - uint32_t member_data_offset, - uint32_t member_data_size); +int vb2_verify_member_inside(const void *parent, size_t parent_size, + const void *member, size_t member_size, + ptrdiff_t member_data_offset, + size_t member_data_size); /** * Verify a signature is fully contained in its parent data diff --git a/firmware/2lib/include/2return_codes.h b/firmware/2lib/include/2return_codes.h index 28c0f91d..f81dc732 100644 --- a/firmware/2lib/include/2return_codes.h +++ b/firmware/2lib/include/2return_codes.h @@ -100,6 +100,32 @@ enum vb2_return_code { VB2_ERROR_SECDATA_SET_FLAGS, /********************************************************************** + * Common code errors + */ + VB2_ERROR_COMMON = VB2_ERROR_BASE + 0x050000, + + /* Buffer is smaller than alignment offset in vb2_align() */ + VB2_ERROR_ALIGN_BIGGER_THAN_SIZE, + + /* Buffer is smaller than request in vb2_align() */ + VB2_ERROR_ALIGN_SIZE, + + /* Parent wraps around in vb2_verify_member_inside() */ + VB2_ERROR_INSIDE_PARENT_WRAPS, + + /* Member wraps around in vb2_verify_member_inside() */ + VB2_ERROR_INSIDE_MEMBER_WRAPS, + + /* Member outside parent in vb2_verify_member_inside() */ + VB2_ERROR_INSIDE_MEMBER_OUTSIDE, + + /* Member data wraps around in vb2_verify_member_inside() */ + VB2_ERROR_INSIDE_DATA_WRAPS, + + /* Member data outside parent in vb2_verify_member_inside() */ + VB2_ERROR_INSIDE_DATA_OUTSIDE, + + /********************************************************************** * TODO: errors which must still be made specific */ VB2_ERROR_TODO = VB2_ERROR_BASE + 0xff0000, diff --git a/tests/vb2_common_tests.c b/tests/vb2_common_tests.c index fd8a7acc..e476edf1 100644 --- a/tests/vb2_common_tests.c +++ b/tests/vb2_common_tests.c @@ -23,27 +23,30 @@ static void test_align(void) p0 = (uint8_t *)buf; ptr = p0; size = 16; - TEST_EQ(vb2_align(&ptr, &size, 4, 16), 0, "vb2_align() aligned"); + TEST_SUCC(vb2_align(&ptr, &size, 4, 16), "vb2_align() aligned"); TEST_EQ(vb2_offset_of(p0, ptr), 0, "ptr"); TEST_EQ(size, 16, " size"); - TEST_NEQ(vb2_align(&ptr, &size, 4, 17), 0, "vb2_align() small"); + TEST_EQ(vb2_align(&ptr, &size, 4, 17), + VB2_ERROR_ALIGN_SIZE, "vb2_align() small"); /* Offset */ ptr = p0 + 1; size = 15; - TEST_EQ(vb2_align(&ptr, &size, 4, 12), 0, "vb2_align() offset"); + TEST_SUCC(vb2_align(&ptr, &size, 4, 12), "vb2_align() offset"); TEST_EQ(vb2_offset_of(p0, ptr), 4, "ptr"); TEST_EQ(size, 12, " size"); /* Offset, now too small */ ptr = p0 + 1; size = 15; - TEST_NEQ(vb2_align(&ptr, &size, 4, 15), 0, "vb2_align() offset small"); + TEST_EQ(vb2_align(&ptr, &size, 4, 15), + VB2_ERROR_ALIGN_SIZE, "vb2_align() offset small"); /* Offset, too small even to align */ ptr = p0 + 1; size = 1; - TEST_NEQ(vb2_align(&ptr, &size, 4, 1), 0, "vb2_align() offset tiny"); + TEST_EQ(vb2_align(&ptr, &size, 4, 1), + VB2_ERROR_ALIGN_BIGGER_THAN_SIZE, "vb2_align() offset tiny"); } /** @@ -149,69 +152,79 @@ static void test_helper_functions(void) { uint8_t *p = (uint8_t *)test_helper_functions; - TEST_EQ(vb2_verify_member_inside(p, 20, p, 6, 11, 3), 0, - "MemberInside ok 1"); - TEST_EQ(vb2_verify_member_inside(p, 20, p+4, 4, 8, 4), 0, - "MemberInside ok 2"); - TEST_NEQ(vb2_verify_member_inside(p, 20, p-4, 4, 8, 4), 0, - "MemberInside member before parent"); - TEST_NEQ(vb2_verify_member_inside(p, 20, p+20, 4, 8, 4), 0, - "MemberInside member after parent"); - TEST_NEQ(vb2_verify_member_inside(p, 20, p, 21, 0, 0), 0, - "MemberInside member too big"); - TEST_NEQ(vb2_verify_member_inside(p, 20, p, 4, 21, 0), 0, - "MemberInside data after parent"); - TEST_NEQ(vb2_verify_member_inside(p, 20, p, 4, UINT32_MAX, 0), - 0, "MemberInside data before parent"); - TEST_NEQ(vb2_verify_member_inside(p, 20, p, 4, 4, 17), 0, - "MemberInside data too big"); - TEST_NEQ(vb2_verify_member_inside(p, UINT32_MAX, - p+UINT32_MAX-9, 12, 5, 0), 0, - "MemberInside wraparound 1"); - TEST_NEQ(vb2_verify_member_inside(p, UINT32_MAX, - p+UINT32_MAX-9, 5, 12, 0), 0, - "MemberInside wraparound 2"); - TEST_NEQ(vb2_verify_member_inside(p, UINT32_MAX, - p+UINT32_MAX-9, 5, 0, 12), 0, - "MemberInside wraparound 3"); + TEST_SUCC(vb2_verify_member_inside(p, 20, p, 6, 11, 3), + "MemberInside ok 1"); + TEST_SUCC(vb2_verify_member_inside(p, 20, p+4, 4, 8, 4), + "MemberInside ok 2"); + TEST_EQ(vb2_verify_member_inside(p, 20, p-4, 4, 8, 4), + VB2_ERROR_INSIDE_MEMBER_OUTSIDE, + "MemberInside member before parent"); + TEST_EQ(vb2_verify_member_inside(p, 20, p+20, 4, 8, 4), + VB2_ERROR_INSIDE_MEMBER_OUTSIDE, + "MemberInside member after parent"); + TEST_EQ(vb2_verify_member_inside(p, 20, p, 21, 0, 0), + VB2_ERROR_INSIDE_MEMBER_OUTSIDE, + "MemberInside member too big"); + TEST_EQ(vb2_verify_member_inside(p, 20, p, 4, 21, 0), + VB2_ERROR_INSIDE_DATA_OUTSIDE, + "MemberInside data after parent"); + TEST_EQ(vb2_verify_member_inside(p, 20, p, 4, SIZE_MAX, 0), + VB2_ERROR_INSIDE_DATA_OUTSIDE, + "MemberInside data before parent"); + TEST_EQ(vb2_verify_member_inside(p, 20, p, 4, 4, 17), + VB2_ERROR_INSIDE_DATA_OUTSIDE, + "MemberInside data too big"); + TEST_EQ(vb2_verify_member_inside(p, -8, p, 12, 0, 0), + VB2_ERROR_INSIDE_PARENT_WRAPS, + "MemberInside wraparound 1"); + TEST_EQ(vb2_verify_member_inside(p, 20, p, -8, 0, 0), + VB2_ERROR_INSIDE_MEMBER_WRAPS, + "MemberInside wraparound 2"); + TEST_EQ(vb2_verify_member_inside(p, 20, p, 4, 0, -8), + VB2_ERROR_INSIDE_DATA_WRAPS, + "MemberInside wraparound 3"); } { struct vb2_packed_key k = {.key_offset = sizeof(k), .key_size = 128}; - TEST_EQ(vb2_verify_packed_key_inside(&k, sizeof(k)+128, &k), 0, - "PublicKeyInside ok 1"); - TEST_EQ(vb2_verify_packed_key_inside(&k - 1, - 2*sizeof(k)+128, &k), - 0, "PublicKeyInside ok 2"); - TEST_NEQ(vb2_verify_packed_key_inside(&k, 128, &k), 0, - "PublicKeyInside key too big"); + TEST_SUCC(vb2_verify_packed_key_inside(&k, sizeof(k)+128, &k), + "PublicKeyInside ok 1"); + TEST_SUCC(vb2_verify_packed_key_inside(&k - 1, + 2*sizeof(k)+128, &k), + "PublicKeyInside ok 2"); + TEST_EQ(vb2_verify_packed_key_inside(&k, 128, &k), + VB2_ERROR_INSIDE_DATA_OUTSIDE, + "PublicKeyInside key too big"); } { struct vb2_packed_key k = {.key_offset = 100, .key_size = 4}; - TEST_NEQ(vb2_verify_packed_key_inside(&k, 99, &k), 0, - "PublicKeyInside offset too big"); + TEST_EQ(vb2_verify_packed_key_inside(&k, 99, &k), + VB2_ERROR_INSIDE_DATA_OUTSIDE, + "PublicKeyInside offset too big"); } { struct vb2_signature s = {.sig_offset = sizeof(s), .sig_size = 128}; - TEST_EQ(vb2_verify_signature_inside(&s, sizeof(s)+128, &s), 0, + TEST_SUCC(vb2_verify_signature_inside(&s, sizeof(s)+128, &s), "SignatureInside ok 1"); - TEST_EQ(vb2_verify_signature_inside(&s - 1, - 2*sizeof(s)+128, &s), - 0, "SignatureInside ok 2"); - TEST_NEQ(vb2_verify_signature_inside(&s, 128, &s), 0, - "SignatureInside sig too big"); + TEST_SUCC(vb2_verify_signature_inside(&s - 1, + 2*sizeof(s)+128, &s), + "SignatureInside ok 2"); + TEST_EQ(vb2_verify_signature_inside(&s, 128, &s), + VB2_ERROR_INSIDE_DATA_OUTSIDE, + "SignatureInside sig too big"); } { struct vb2_signature s = {.sig_offset = 100, .sig_size = 4}; - TEST_NEQ(vb2_verify_signature_inside(&s, 99, &s), 0, - "SignatureInside offset too big"); + TEST_EQ(vb2_verify_signature_inside(&s, 99, &s), + VB2_ERROR_INSIDE_DATA_OUTSIDE, + "SignatureInside offset too big"); } } |