Merge pull request #732 from danielinux/fixes-20260322

dgarske · web-flow · commit 17df962a06e7 · 2026-03-23T09:31:31.000-07:00
Fixes from static analysis
diff --git a/.gitignore b/.gitignore
@@ -170,6 +170,7 @@ tools/unit-tests/unit-image-sha384
 tools/unit-tests/unit-store-sbrk
 tools/unit-tests/unit-tpm-blob
 tools/unit-tests/unit-update-disk
+tools/unit-tests/unit-policy-sign
 
 
 
diff --git a/src/boot_arm32.c b/src/boot_arm32.c
@@ -86,7 +86,7 @@ void RAMFUNCTION do_boot(const uint32_t *app_offset)
 #ifdef RAM_CODE
 
 #define AIRCR *(volatile uint32_t *)(0xE000ED0C)
-#define AIRCR_VKEY (0r05FA << 16)
+#define AIRCR_VKEY (0x05FA << 16)
 #define AIRCR_SYSRESETREQ (1 << 2)
 
 void RAMFUNCTION arch_reboot(void)
diff --git a/src/pkcs11_store.c b/src/pkcs11_store.c
@@ -436,9 +436,7 @@ int wolfPKCS11_Store_Open(int type, CK_ULONG id1, CK_ULONG id2, int read,
 void wolfPKCS11_Store_Close(void* store)
 {
     struct store_handle *handle = store;
-    /* This removes all flags (including STORE_FLAGS_OPEN) */
-    handle->flags = 0;
-    handle->hdr = NULL;
+    memset(handle, 0, sizeof(*handle));
 }
 
 int wolfPKCS11_Store_Read(void* store, unsigned char* buffer, int len)
diff --git a/src/psa_store.c b/src/psa_store.c
@@ -442,9 +442,7 @@ int wolfPSA_Store_OpenSz(int type, unsigned long id1, unsigned long id2, int rea
 void wolfPSA_Store_Close(void* store)
 {
     struct store_handle *handle = store;
-    /* This removes all flags (including STORE_FLAGS_OPEN) */
-    handle->flags = 0;
-    handle->hdr = NULL;
+    memset(handle, 0, sizeof(*handle));
 }
 
 int wolfPSA_Store_Read(void* store, unsigned char* buffer, int len)
diff --git a/src/update_disk.c b/src/update_disk.c
@@ -221,6 +221,11 @@ static void disk_crypto_clear(void)
     ForceZero(disk_encrypt_nonce, sizeof(disk_encrypt_nonce));
 }
 
+static void disk_decrypted_header_clear(uint8_t *hdr)
+{
+    ForceZero(hdr, IMAGE_HEADER_SIZE);
+}
+
 #endif /* DISK_ENCRYPT */
 
 extern int wolfBoot_get_dts_size(void *dts_addr);
@@ -267,12 +272,14 @@ void RAMFUNCTION wolfBoot_start(void)
 #ifdef DISK_ENCRYPT
     /* Initialize encryption - this sets up the cipher with key from storage */
     if (wolfBoot_initialize_encryption() != 0) {
+        disk_decrypted_header_clear(dec_hdr);
         disk_crypto_clear();
         wolfBoot_printf("Error initializing encryption\r\n");
         wolfBoot_panic();
     }
     /* Retrieve encryption key and nonce for disk decryption */
     if (wolfBoot_get_encrypt_key(disk_encrypt_key, disk_encrypt_nonce) != 0) {
+        disk_decrypted_header_clear(dec_hdr);
         disk_crypto_clear();
         wolfBoot_printf("Error getting encryption key\r\n");
         wolfBoot_panic();
@@ -283,13 +290,15 @@ void RAMFUNCTION wolfBoot_start(void)
     ret = disk_init(BOOT_DISK);
     if (ret != 0) {
 #ifdef DISK_ENCRYPT
+        disk_decrypted_header_clear(dec_hdr);
         disk_crypto_clear();
 #endif
         wolfBoot_panic();
     }
 
     if (disk_open(BOOT_DISK) < 0) {
 #ifdef DISK_ENCRYPT
+        disk_decrypted_header_clear(dec_hdr);
         disk_crypto_clear();
 #endif
         wolfBoot_printf("Error opening disk %d\r\n", BOOT_DISK);
@@ -328,6 +337,7 @@ void RAMFUNCTION wolfBoot_start(void)
 
     if ((pB_ver == 0) && (pA_ver == 0)) {
 #ifdef DISK_ENCRYPT
+        disk_decrypted_header_clear(dec_hdr);
         disk_crypto_clear();
 #endif
         wolfBoot_printf("No valid OS image found in either partition %d or %d\r\n",
@@ -433,6 +443,7 @@ void RAMFUNCTION wolfBoot_start(void)
         wolfBoot_printf("Decrypting image...");
         BENCHMARK_START();
         if ((IMAGE_HEADER_SIZE % ENCRYPT_BLOCK_SIZE) != 0) {
+            disk_decrypted_header_clear(dec_hdr);
             disk_crypto_clear();
             wolfBoot_printf("Encrypted disk images require aligned header size\r\n");
             wolfBoot_panic();
@@ -482,6 +493,7 @@ void RAMFUNCTION wolfBoot_start(void)
 
     if (failures) {
 #ifdef DISK_ENCRYPT
+        disk_decrypted_header_clear(dec_hdr);
         disk_crypto_clear();
 #endif
         wolfBoot_printf("Unable to find a valid partition!\r\n");
@@ -542,6 +554,7 @@ void RAMFUNCTION wolfBoot_start(void)
     wolfBoot_hook_boot(&os_image);
 #endif
 #ifdef DISK_ENCRYPT
+    disk_decrypted_header_clear(dec_hdr);
     disk_crypto_clear();
 #endif
     do_boot((uint32_t*)load_address
diff --git a/src/update_flash.c b/src/update_flash.c
@@ -1241,7 +1241,6 @@ int wolfBoot_unlock_disk(void)
             ret = wolfBoot_get_random(secret, secretSz);
             if (ret == 0) {
                 wolfBoot_printf("Creating new secret (%d bytes)\n", secretSz);
-                wolfBoot_print_hexstr(secret, secretSz, 0);
 
                 /* seal new secret */
                 ret = wolfBoot_seal(pubkey_hint, policy, policySz, nvIndex,
@@ -1265,15 +1264,13 @@ int wolfBoot_unlock_disk(void)
                 }
 
                 wolfBoot_printf("Secret Check %d bytes\n", secretCheckSz);
-                wolfBoot_print_hexstr(secretCheck, secretCheckSz, 0);
                 TPM2_ForceZero(secretCheck, sizeof(secretCheck));
             }
         }
     }
 
     if (ret == 0) {
         wolfBoot_printf("Secret %d bytes\n", secretSz);
-        wolfBoot_print_hexstr(secret, secretSz, 0);
 
         /* TODO: Unlock disk */
 
diff --git a/src/x86/ahci.c b/src/x86/ahci.c
@@ -281,7 +281,6 @@ static int sata_create_and_seal_unlock_secret(const uint8_t *pubkey_hint,
     ret = sata_get_random_base64(secret, secret_size);
     if (ret == 0) {
         wolfBoot_printf("Creating new secret (%d bytes)\r\n", *secret_size);
-        wolfBoot_printf("%s\r\n", secret);
 
         /* seal new secret */
         ret = wolfBoot_seal(pubkey_hint, policy, policy_size,
@@ -305,14 +304,11 @@ static int sata_create_and_seal_unlock_secret(const uint8_t *pubkey_hint,
         }
 
         wolfBoot_printf("Secret Check %d bytes\n", secret_check_sz);
-        wolfBoot_printf("%s\r\n", secret_check);
         TPM2_ForceZero(secret_check, sizeof(secret_check));
     }
 
-    if (ret == 0) {
+    if (ret == 0)
         wolfBoot_printf("Secret %d bytes\n", *secret_size);
-        wolfBoot_printf("%s\r\n", secret);
-    }
 
     return ret;
 }
@@ -414,9 +410,6 @@ int sata_unlock_disk(int drv, int freeze)
     r = sata_get_unlock_secret(secret, &secret_size);
     if (r != 0)
         return r;
-#ifdef TARGET_x86_fsp_qemu
-    wolfBoot_printf("DISK LOCK SECRET: %s\r\n", secret);
-#endif
     ata_st = ata_security_get_state(drv);
     wolfBoot_printf("ATA: Security state SEC%d\r\n", ata_st);
 #if defined(TARGET_x86_fsp_qemu)
diff --git a/tools/elf-parser/elf-parser.c b/tools/elf-parser/elf-parser.c
@@ -67,7 +67,9 @@ int main(int argc, char *argv[])
             ret = -1;
         }
     }
-    fclose(f);
+    if (f != NULL) {
+        fclose(f);
+    }
 
     if (ret == 0) {
         ret = elf_load_image_mmu(image, (uint32_t)imageSz, &entry, NULL);
diff --git a/tools/fdt-parser/fdt-parser.c b/tools/fdt-parser/fdt-parser.c
@@ -192,10 +192,18 @@ static int fdt_test(void* fdt)
     off = fdt_node_offset_by_compatible(fdt, -1, "fsl,qman-portal");
     while (off != -FDT_ERR_NOTFOUND) {
         const int *ci = fdt_getprop(fdt, off, "cell-index", NULL);
+        uint32_t portal_idx;
         uint32_t liodns[2];
         if (!ci)
             break;
-        i = fdt32_to_cpu(*ci);
+        portal_idx = fdt32_to_cpu(*ci);
+        if (portal_idx >= QMAN_NUM_PORTALS) {
+            printf("FDT: Invalid qman-portal cell-index %u at %d\n",
+                portal_idx, off);
+            ret = -FDT_ERR_BADSTRUCTURE;
+            goto exit;
+        }
+        i = (int)portal_idx;
 
         liodns[0] = qp_info[i].dliodn;
         liodns[1] = qp_info[i].fliodn;
diff --git a/tools/tpm/policy_sign.c b/tools/tpm/policy_sign.c
@@ -287,7 +287,7 @@ int policy_sign(int argc, char *argv[])
                 pcrDigestSz = -1;
             else
                 pcrDigestSz = hexToByte(hashHexStr, pcrDigest, hashHexStrlen);
-            if (pcrDigestSz <= 0) {
+            if ((int)pcrDigestSz <= 0) {
                 fprintf(stderr, "Invalid PCR hash length\n");
                 usage();
                 return -1;
@@ -300,7 +300,7 @@ int policy_sign(int argc, char *argv[])
                 digestSz = -1;
             else
                 digestSz = hexToByte(hashHexStr, digest, hashHexStrlen);
-            if (digestSz <= 0) {
+            if ((int)digestSz <= 0) {
                 fprintf(stderr, "Invalid Policy Digest hash length\n");
                 usage();
                 return -1;
diff --git a/tools/unit-tests/Makefile b/tools/unit-tests/Makefile
@@ -49,7 +49,7 @@ TESTS:=unit-parser unit-extflash unit-string unit-spi-flash unit-aes128 \
        unit-update-flash-enc unit-update-ram unit-pkcs11_store unit-psa_store unit-disk \
        unit-update-disk unit-multiboot unit-boot-x86-fsp unit-qspi-flash unit-tpm-rsa-exp \
        unit-image-nopart unit-image-sha384 unit-image-sha3-384 unit-store-sbrk \
-       unit-tpm-blob
+       unit-tpm-blob unit-policy-sign
 
 all: $(TESTS)
 
@@ -132,6 +132,13 @@ unit-tpm-blob: ../../include/target.h unit-tpm-blob.c
 		-DWOLFBOOT_HASH_SHA256 \
 		-ffunction-sections -fdata-sections $(LDFLAGS) -Wl,--gc-sections
 
+unit-policy-sign: ../../include/target.h unit-policy-sign.c \
+		$(WOLFBOOT_LIB_WOLFSSL)/wolfcrypt/src/memory.c
+	gcc -o $@ $^ -I../tpm $(CFLAGS) -I$(WOLFBOOT_LIB_WOLFTPM) -DWOLFBOOT_TPM \
+		-DWOLFTPM_USER_SETTINGS -DWOLFBOOT_SIGN_ECC256 -DWOLFBOOT_HASH_SHA256 \
+		-DHAVE_ECC_KEY_IMPORT \
+		-ffunction-sections -fdata-sections $(LDFLAGS) -Wl,--gc-sections
+
 unit-store-sbrk: unit-store-sbrk.c ../../src/store_sbrk.c
 	gcc -o $@ $^ $(CFLAGS) $(LDFLAGS)
 
diff --git a/tools/unit-tests/unit-pkcs11_store.c b/tools/unit-tests/unit-pkcs11_store.c
@@ -318,6 +318,39 @@ START_TEST(test_cross_sector_write_preserves_length)
 }
 END_TEST
 
+START_TEST(test_close_clears_handle_state)
+{
+    const int type = DYNAMIC_TYPE_RSA;
+    const CK_ULONG id_tok = 17;
+    const CK_ULONG id_obj = 21;
+    void *store = NULL;
+    struct store_handle *handle;
+    int ret;
+
+    ret = mmap_file("/tmp/wolfboot-unit-keyvault.bin", vault_base,
+            keyvault_size, NULL);
+    ck_assert_int_eq(ret, 0);
+    memset(vault_base, 0xEE, keyvault_size);
+
+    ret = wolfPKCS11_Store_Open(type, id_tok, id_obj, 0, &store);
+    ck_assert_int_eq(ret, 0);
+    ck_assert_ptr_nonnull(store);
+
+    handle = store;
+    ck_assert_ptr_nonnull(handle->buffer);
+    ck_assert_ptr_nonnull(handle->hdr);
+    ck_assert_uint_ne(handle->in_buffer_offset, 0);
+
+    wolfPKCS11_Store_Close(store);
+
+    ck_assert_uint_eq(handle->flags, 0);
+    ck_assert_uint_eq(handle->pos, 0);
+    ck_assert_ptr_null(handle->buffer);
+    ck_assert_ptr_null(handle->hdr);
+    ck_assert_uint_eq(handle->in_buffer_offset, 0);
+}
+END_TEST
+
 START_TEST(test_delete_object_ignores_metadata_prefix)
 {
     const int32_t type = DYNAMIC_TYPE_RSA;
@@ -356,12 +389,15 @@ Suite *wolfboot_suite(void)
 
     TCase* tcase_store_and_load_objs = tcase_create("store_and_load_objs");
     TCase* tcase_cross_sector_write = tcase_create("cross_sector_write");
+    TCase* tcase_close = tcase_create("close_state");
     TCase* tcase_delete_object = tcase_create("delete_object");
     tcase_add_test(tcase_store_and_load_objs, test_store_and_load_objs);
     tcase_add_test(tcase_cross_sector_write, test_cross_sector_write_preserves_length);
+    tcase_add_test(tcase_close, test_close_clears_handle_state);
     tcase_add_test(tcase_delete_object, test_delete_object_ignores_metadata_prefix);
     suite_add_tcase(s, tcase_store_and_load_objs);
     suite_add_tcase(s, tcase_cross_sector_write);
+    suite_add_tcase(s, tcase_close);
     suite_add_tcase(s, tcase_delete_object);
     return s;
 }
diff --git a/tools/unit-tests/unit-policy-sign.c b/tools/unit-tests/unit-policy-sign.c
diff --git a/tools/unit-tests/unit-psa_store.c b/tools/unit-tests/unit-psa_store.c

Original file line number	Diff line number	Diff line change
`@@ -436,9 +436,7 @@ int wolfPKCS11_Store_Open(int type, CK_ULONG id1, CK_ULONG id2, int read,`
`436`	`436`	`void wolfPKCS11_Store_Close(void* store)`
`437`	`437`	`{`
`438`	`438`	`struct store_handle *handle = store;`
`439`		`- /* This removes all flags (including STORE_FLAGS_OPEN) */`
`440`		`- handle->flags = 0;`
`441`		`- handle->hdr = NULL;`
	`439`	`+ memset(handle, 0, sizeof(*handle));`
`442`	`440`	`}`
`443`	`441`
`444`	`442`	`int wolfPKCS11_Store_Read(void* store, unsigned char* buffer, int len)`
Original file line number	Diff line number	Diff line change
`@@ -442,9 +442,7 @@ int wolfPSA_Store_OpenSz(int type, unsigned long id1, unsigned long id2, int rea`
`442`	`442`	`void wolfPSA_Store_Close(void* store)`
`443`	`443`	`{`
`444`	`444`	`struct store_handle *handle = store;`
`445`		`- /* This removes all flags (including STORE_FLAGS_OPEN) */`
`446`		`- handle->flags = 0;`
`447`		`- handle->hdr = NULL;`
	`445`	`+ memset(handle, 0, sizeof(*handle));`
`448`	`446`	`}`
`449`	`447`
`450`	`448`	`int wolfPSA_Store_Read(void* store, unsigned char* buffer, int len)`
Original file line number	Diff line number	Diff line change
`@@ -1241,7 +1241,6 @@ int wolfBoot_unlock_disk(void)`
`1241`	`1241`	`ret = wolfBoot_get_random(secret, secretSz);`
`1242`	`1242`	`if (ret == 0) {`
`1243`	`1243`	`wolfBoot_printf("Creating new secret (%d bytes)\n", secretSz);`
`1244`		`- wolfBoot_print_hexstr(secret, secretSz, 0);`
`1245`	`1244`
`1246`	`1245`	`/* seal new secret */`
`1247`	`1246`	`ret = wolfBoot_seal(pubkey_hint, policy, policySz, nvIndex,`
`@@ -1265,15 +1264,13 @@ int wolfBoot_unlock_disk(void)`
`1265`	`1264`	`}`
`1266`	`1265`
`1267`	`1266`	`wolfBoot_printf("Secret Check %d bytes\n", secretCheckSz);`
`1268`		`- wolfBoot_print_hexstr(secretCheck, secretCheckSz, 0);`
`1269`	`1267`	`TPM2_ForceZero(secretCheck, sizeof(secretCheck));`
`1270`	`1268`	`}`
`1271`	`1269`	`}`
`1272`	`1270`	`}`
`1273`	`1271`
`1274`	`1272`	`if (ret == 0) {`
`1275`	`1273`	`wolfBoot_printf("Secret %d bytes\n", secretSz);`
`1276`		`- wolfBoot_print_hexstr(secret, secretSz, 0);`
`1277`	`1274`
`1278`	`1275`	`/* TODO: Unlock disk */`
`1279`	`1276`
Original file line number	Diff line number	Diff line change
`@@ -67,7 +67,9 @@ int main(int argc, char *argv[])`
`67`	`67`	`ret = -1;`
`68`	`68`	`}`
`69`	`69`	`}`
`70`		`- fclose(f);`
	`70`	`+ if (f != NULL) {`
	`71`	`+ fclose(f);`
	`72`	`+ }`
`71`	`73`
`72`	`74`	`if (ret == 0) {`
`73`	`75`	`ret = elf_load_image_mmu(image, (uint32_t)imageSz, &entry, NULL);`