[coreboot-gerrit] Change in coreboot[master]: mainboard/emulation/qemu-i440fx/fw_cfg: Fix undefined behavior

Ryan Salsamendi (Code Review) gerrit at coreboot.org
Mon Jun 12 04:43:21 CEST 2017


Ryan Salsamendi has uploaded this change for review. ( https://review.coreboot.org/20155


Change subject: mainboard/emulation/qemu-i440fx/fw_cfg: Fix undefined behavior
......................................................................

mainboard/emulation/qemu-i440fx/fw_cfg: Fix undefined behavior

Fixes 2 reports found by undefined behavior sanitizer. Dereferencing
pointers that are not aligned to the size of access is undefiend
behavior.

Change-Id: Iaa3845308171c307f1ddc7937286aacbd00e3a10
Signed-off-by: Ryan Salsamendi <rsalsamendi at hotmail.com>
---
M src/mainboard/emulation/qemu-i440fx/fw_cfg.c
1 file changed, 14 insertions(+), 6 deletions(-)



  git pull ssh://review.coreboot.org:29418/coreboot refs/changes/55/20155/1

diff --git a/src/mainboard/emulation/qemu-i440fx/fw_cfg.c b/src/mainboard/emulation/qemu-i440fx/fw_cfg.c
index 565b855..19a1571 100644
--- a/src/mainboard/emulation/qemu-i440fx/fw_cfg.c
+++ b/src/mainboard/emulation/qemu-i440fx/fw_cfg.c
@@ -220,6 +220,10 @@
 
 	current = start;
 	for (i = 0; i < max && s[i].command != 0; i++) {
+		void *cksum_data;
+		uint32_t cksum;
+		uint32_t addr4;
+		uint64_t addr8;
 		switch (s[i].command) {
 		case BIOS_LINKER_LOADER_COMMAND_ALLOCATE:
 			current = ALIGN(current, s[i].alloc.align);
@@ -249,12 +253,16 @@
 			switch (s[i].pointer.size) {
 			case 4:
 				ptr4 = (uint32_t*)(addrs[dst] + s[i].pointer.offset);
-				*ptr4 += addrs[src];
+				memcpy(&addr4, ptr4, sizeof(addr4));
+				addr4 += addrs[src];
+				memcpy(ptr4, &addr4, sizeof(addr4));
 				break;
 
 			case 8:
 				ptr8 = (uint64_t*)(addrs[dst] + s[i].pointer.offset);
-				*ptr8 += addrs[src];
+				memcpy(&addr8, ptr8, sizeof(addr8));
+				addr8 += addrs[src];
+				memcpy(ptr8, &addr8, sizeof(addr8));
 				break;
 
 			default:
@@ -280,10 +288,10 @@
 			if (dst == -1)
 				goto err;
 
-			ptr4 = (uint32_t*)(addrs[dst] + s[i].cksum.offset);
-			*ptr4 = 0;
-			*ptr4 = acpi_checksum((void *)(addrs[dst] + s[i].cksum.start),
-					      s[i].cksum.length);
+			ptr4 = (uint32_t *)(addrs[dst] + s[i].cksum.offset);
+			cksum_data = (void *)(addrs[dst] + s[i].cksum.start);
+			cksum = acpi_checksum(cksum_data, s[i].cksum.length);
+			memcpy(ptr4, &cksum, sizeof(cksum));
 			break;
 
 		default:

-- 
To view, visit https://review.coreboot.org/20155
To unsubscribe, visit https://review.coreboot.org/settings

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Iaa3845308171c307f1ddc7937286aacbd00e3a10
Gerrit-Change-Number: 20155
Gerrit-PatchSet: 1
Gerrit-Owner: Ryan Salsamendi <rsalsamendi at hotmail.com>



More information about the coreboot-gerrit mailing list