multiboot2 fixes + tests

src/multiboot.c

@@ -23,6 +23,7 @@
 
 #include <stdint.h>
 #include <stdio.h>
+#include <string.h>
 #include <printf.h>
 #include <sys/types.h>
 
@@ -102,11 +103,17 @@ static uint8_t* mb2_align_address_up(uint8_t *addr, int align)
     return (uint8_t*)((v + mask) & ~(mask));
 }
 
-static uint8_t *mb2_find_tag_by_type(uint8_t *tags, uint32_t type)
+static uint8_t *mb2_find_tag_by_type(uint8_t *tags, uint32_t tags_len,
+                                     uint32_t type)
 {
+    uint8_t *end = tags + tags_len;
     struct mb2_tag* tag = (struct mb2_tag*)tags;
 
-    while (tag->type != 0) {
+    while ((uint8_t*)tag + sizeof(*tag) <= end && tag->type != 0) {
+        if (tag->size < sizeof(*tag))
+            return NULL;
+        if (tag->size > ((uint32_t)(end - (uint8_t*)tag)))
+            return NULL;
         if (tag->type == type)
             return (uint8_t*)tag;
         tag = (struct mb2_tag*)mb2_align_address_up((uint8_t*)tag + tag->size,
@@ -247,6 +254,7 @@ int mb2_build_boot_info_header(uint8_t *mb2_boot_info,
         (struct mb2_boot_info_header *)mb2_boot_info;
     struct mb2_tag_info_req *info_req_tag;
     int requested_tags, i, r;
+    uint32_t header_length;
     uint8_t *idx;
 
     if (max_size < sizeof(*hdr)) {
@@ -256,8 +264,14 @@ int mb2_build_boot_info_header(uint8_t *mb2_boot_info,
     max_size -= sizeof(*hdr);
     idx = (uint8_t*)hdr + sizeof(*hdr);
     hdr->reserved = 0;
+    header_length = ((struct mb2_header *)mb2_header)->header_length;
+    if (header_length < sizeof(struct mb2_header))
+        return -1;
     info_req_tag =
-        (struct mb2_tag_info_req *)mb2_find_tag_by_type(mb2_header + sizeof(struct mb2_header), MB2_TAG_TYPE_INFO_REQ);
+        (struct mb2_tag_info_req *)mb2_find_tag_by_type(
+            mb2_header + sizeof(struct mb2_header),
+            header_length - sizeof(struct mb2_header),
+            MB2_TAG_TYPE_INFO_REQ);
     if (info_req_tag == NULL)
         return -1;
     requested_tags = (info_req_tag->size - sizeof(struct mb2_tag_info_req)) / sizeof(uint32_t);
@@ -274,7 +288,8 @@ int mb2_build_boot_info_header(uint8_t *mb2_boot_info,
                 return r;
             break;
         default:
-            wolfBoot_printf("mb2: unsupported info request tag: %d\r\n", i);
+            wolfBoot_printf("mb2: unsupported info request tag: %d\r\n",
+                          info_req_tag->mbi_tag_types[i]);
             return -1;
         }
     }
@@ -303,14 +318,21 @@ static void mb2_parse_info_request_tag(void* tag) {
     }
 }
 
-static void mb2_dump_tags(void* mbTags) {
+static void mb2_dump_tags(void* mbTags, uint32_t tags_len) {
     struct mb2_tag* tag = (struct  mb2_tag*)mbTags;
+    uint8_t *end = (uint8_t*)mbTags + tags_len;
 
-    while (tag->type != 0) {
+    while ((uint8_t*)tag + sizeof(*tag) <= end && tag->type != 0) {
         MB2_DEBUG_PRINTF("Tag Type: %u\r\n", tag->type);
         MB2_DEBUG_PRINTF("Tag Flags: 0x%x\r\n", tag->flags);
         MB2_DEBUG_PRINTF("Tag Size: %u\r\n", tag->size);
 
+        if (tag->size < sizeof(*tag))
+            return;
+
+        if (tag->size > ((uint32_t)(end - (uint8_t*)tag)))
+            return;
+
         if (tag->type == MB2_TAG_TYPE_INFO_REQ)
             mb2_parse_info_request_tag(tag);
 
@@ -329,24 +351,27 @@ static void mb2_dump_header(void* mbHeader) {
     MB2_DEBUG_PRINTF("Checksum: 0x%x\r\n", header->checksum);
 
     tags = (uint8_t*)header + sizeof(*header);
-    mb2_dump_tags(tags);
+    if (header->header_length < sizeof(struct mb2_header))
+        MB2_DEBUG_PRINTF("Invalid header length\r\n");
+    mb2_dump_tags(tags, header->header_length - sizeof(*header));
 }
 #endif /* DEBUG_MB2 */
 
 uint8_t *mb2_find_header(uint8_t *image, int size)
 {
-    uint32_t *ptr;
+    uint32_t val;
     int i;
 
-    if (size > MB2_HEADER_MAX_OFF/4)
+    if (size > MB2_HEADER_MAX_OFF)
         size = MB2_HEADER_MAX_OFF;
     size = size / 4;
-    for (ptr = (uint32_t*)image,i = 0; i < size; ++i) {
-        if (ptr[i] == MB2_HEADER_MAGIC) {
+    for (i = 0; i < size; ++i) {
+        memcpy(&val, image + i * 4, sizeof(val));
+        if (val == MB2_HEADER_MAGIC) {
 #ifdef DEBUG_MB2
-            mb2_dump_header(&ptr[i]);
+            mb2_dump_header(image + i * 4);
 #endif /* DEBUG_MB2 */
-            return (uint8_t*)&ptr[i];
+            return image + i * 4;
         }
     }
     return NULL;

test-app/app_x86_fsp_qemu.c

@@ -50,7 +50,7 @@ __attribute__((aligned(8))) struct multiboot_header mbh = {
     .hdr.magic = 0xe85250d6,
     .hdr.architecture = 0,
     .hdr.checksum = 0,
-    .hdr.header_length = sizeof(struct mb2_header),
+    .hdr.header_length = sizeof(struct multiboot_header),
     .req.type = 1,
     .req.flags = 0,
     .req.size = sizeof(struct mb2_tag_info_req),

tools/unit-tests/Makefile

@@ -29,7 +29,8 @@ TESTS:=unit-parser unit-extflash unit-string unit-spi-flash unit-aes128 \
        unit-aes256 unit-chacha20 unit-pci unit-mock-state unit-sectorflags \
        unit-image unit-nvm unit-nvm-flagshome unit-enc-nvm \
        unit-enc-nvm-flagshome unit-delta unit-update-flash \
-       unit-update-flash-enc unit-update-ram unit-pkcs11_store unit-disk
+       unit-update-flash-enc unit-update-ram unit-pkcs11_store unit-disk \
+       unit-multiboot
 
 all: $(TESTS)
 
@@ -165,6 +166,9 @@ gpt-sfdisk-test.h:
 unit-disk: unit-disk.c gpt-sfdisk-test.h
 	gcc -o $@ $< $(CFLAGS) $(LDFLAGS)
 
+unit-multiboot: unit-multiboot.c
+	gcc -o $@ unit-multiboot.c $(CFLAGS) $(LDFLAGS)
+
 %.o:%.c
 	gcc -c -o $@ $^ $(CFLAGS)