[coreboot-gerrit] Patch set updated for coreboot: 23cb6b5 Replace all occurences of sprintf with snprintf

Ronald G. Minnich (rminnich@gmail.com) gerrit at coreboot.org
Wed Dec 25 01:22:59 CET 2013


Ronald G. Minnich (rminnich at gmail.com) just uploaded a new patch set to gerrit, which you can find at http://review.coreboot.org/4279

-gerrit

commit 23cb6b5380dadee388959216a359b12fe53d8623
Author: Vladimir Serbinenko <phcoder at gmail.com>
Date:   Tue Nov 26 02:41:26 2013 +0100

    Replace all occurences of sprintf with snprintf
    
    This reduces risks of buffer overflows.
    
    Change-Id: I77f80e76efec16ac0a0af83d76430a8126a7602d
    Signed-off-by: Vladimir Serbinenko <phcoder at gmail.com>
---
 src/arch/x86/boot/acpigen.c                        |  3 +-
 src/device/device_util.c                           | 83 ++++++++++++----------
 src/device/oprom/x86emu/debug.c                    |  8 +--
 src/ec/lenovo/h8/h8.c                              |  4 +-
 src/mainboard/google/link/i915.c                   |  2 +-
 src/mainboard/google/stout/i915.c                  |  2 +-
 src/mainboard/intel/wtm2/i915.c                    |  2 +-
 src/mainboard/lenovo/x60/i915.c                    |  2 +-
 src/northbridge/amd/agesa/family10/northbridge.c   |  2 +-
 src/northbridge/amd/agesa/family12/northbridge.c   |  2 +-
 src/northbridge/amd/agesa/family14/northbridge.c   |  2 +-
 src/northbridge/amd/agesa/family15/northbridge.c   |  2 +-
 src/northbridge/amd/agesa/family15tn/northbridge.c |  2 +-
 src/northbridge/amd/agesa/family16kb/northbridge.c |  2 +-
 src/northbridge/amd/amdfam10/acpi.c                |  2 +-
 src/northbridge/amd/amdfam10/northbridge.c         |  4 +-
 src/northbridge/amd/amdk8/northbridge.c            |  4 +-
 src/southbridge/intel/lynxpoint/acpi.c             |  2 +-
 18 files changed, 70 insertions(+), 60 deletions(-)

diff --git a/src/arch/x86/boot/acpigen.c b/src/arch/x86/boot/acpigen.c
index 121cb22..6496d75 100644
--- a/src/arch/x86/boot/acpigen.c
+++ b/src/arch/x86/boot/acpigen.c
@@ -274,7 +274,8 @@ int acpigen_write_processor(u8 cpuindex, u32 pblock_addr, u8 pblock_len)
 	acpigen_emit_byte(0x83);
 	len = acpigen_write_len_f();
 
-	sprintf(pscope, "\\_PR.CPU%x", (unsigned int) cpuindex);
+	snprintf(pscope, sizeof (pscope),
+		 "\\_PR.CPU%x", (unsigned int) cpuindex);
 	len += acpigen_emit_namestring(pscope);
 	acpigen_emit_byte(cpuindex);
 	acpigen_emit_byte(pblock_addr & 0xff);
diff --git a/src/device/device_util.c b/src/device/device_util.c
index cad2a06..38654fa 100644
--- a/src/device/device_util.c
+++ b/src/device/device_util.c
@@ -231,48 +231,52 @@ const char *dev_path(device_t dev)
 			break;
 		case DEVICE_PATH_PCI:
 #if CONFIG_PCI_BUS_SEGN_BITS
-			sprintf(buffer, "PCI: %04x:%02x:%02x.%01x",
-				dev->bus->secondary >> 8,
-				dev->bus->secondary & 0xff,
-				PCI_SLOT(dev->path.pci.devfn),
-				PCI_FUNC(dev->path.pci.devfn));
+			snprintf(buffer, sizeof (buffer),
+				 "PCI: %04x:%02x:%02x.%01x",
+				 dev->bus->secondary >> 8,
+				 dev->bus->secondary & 0xff,
+				 PCI_SLOT(dev->path.pci.devfn),
+				 PCI_FUNC(dev->path.pci.devfn));
 #else
-			sprintf(buffer, "PCI: %02x:%02x.%01x",
-				dev->bus->secondary,
-				PCI_SLOT(dev->path.pci.devfn),
-				PCI_FUNC(dev->path.pci.devfn));
+			snprintf(buffer, sizeof (buffer),
+				 "PCI: %02x:%02x.%01x",
+				 dev->bus->secondary,
+				 PCI_SLOT(dev->path.pci.devfn),
+				 PCI_FUNC(dev->path.pci.devfn));
 #endif
 			break;
 		case DEVICE_PATH_PNP:
-			sprintf(buffer, "PNP: %04x.%01x",
-				dev->path.pnp.port, dev->path.pnp.device);
+			snprintf(buffer, sizeof (buffer), "PNP: %04x.%01x",
+				 dev->path.pnp.port, dev->path.pnp.device);
 			break;
 		case DEVICE_PATH_I2C:
-			sprintf(buffer, "I2C: %02x:%02x",
-				dev->bus->secondary,
-				dev->path.i2c.device);
+			snprintf(buffer, sizeof (buffer), "I2C: %02x:%02x",
+				 dev->bus->secondary,
+				 dev->path.i2c.device);
 			break;
 		case DEVICE_PATH_APIC:
-			sprintf(buffer, "APIC: %02x",
-				dev->path.apic.apic_id);
+			snprintf(buffer, sizeof (buffer), "APIC: %02x",
+				 dev->path.apic.apic_id);
 			break;
 		case DEVICE_PATH_IOAPIC:
-			sprintf(buffer, "IOAPIC: %02x",
-				dev->path.ioapic.ioapic_id);
+			snprintf(buffer, sizeof (buffer), "IOAPIC: %02x",
+				 dev->path.ioapic.ioapic_id);
 			break;
 		case DEVICE_PATH_DOMAIN:
-			sprintf(buffer, "DOMAIN: %04x",
+			snprintf(buffer, sizeof (buffer), "DOMAIN: %04x",
 				dev->path.domain.domain);
 			break;
 		case DEVICE_PATH_CPU_CLUSTER:
-			sprintf(buffer, "CPU_CLUSTER: %01x",
+			snprintf(buffer, sizeof (buffer), "CPU_CLUSTER: %01x",
 				dev->path.cpu_cluster.cluster);
 			break;
 		case DEVICE_PATH_CPU:
-			sprintf(buffer, "CPU: %02x", dev->path.cpu.id);
+			snprintf(buffer, sizeof (buffer),
+				 "CPU: %02x", dev->path.cpu.id);
 			break;
 		case DEVICE_PATH_CPU_BUS:
-			sprintf(buffer, "CPU_BUS: %02x", dev->path.cpu_bus.id);
+			snprintf(buffer, sizeof (buffer),
+				 "CPU_BUS: %02x", dev->path.cpu_bus.id);
 			break;
 		default:
 			printk(BIOS_ERR, "Unknown device path type: %d\n",
@@ -296,7 +300,8 @@ const char *dev_name(device_t dev)
 const char *bus_path(struct bus *bus)
 {
 	static char buffer[BUS_PATH_MAX];
-	sprintf(buffer, "%s,%d", dev_path(bus->dev), bus->link_num);
+	snprintf(buffer, sizeof (buffer),
+		 "%s,%d", dev_path(bus->dev), bus->link_num);
 	return buffer;
 }
 
@@ -584,15 +589,15 @@ resource_t resource_max(struct resource *resource)
 const char *resource_type(struct resource *resource)
 {
 	static char buffer[RESOURCE_TYPE_MAX];
-	sprintf(buffer, "%s%s%s%s",
-		((resource->flags & IORESOURCE_READONLY) ? "ro" : ""),
-		((resource->flags & IORESOURCE_PREFETCH) ? "pref" : ""),
-		((resource->flags == 0) ? "unused" :
-		(resource->flags & IORESOURCE_IO) ? "io" :
-		(resource->flags & IORESOURCE_DRQ) ? "drq" :
-		(resource->flags & IORESOURCE_IRQ) ? "irq" :
-		(resource->flags & IORESOURCE_MEM) ? "mem" : "??????"),
-		((resource->flags & IORESOURCE_PCI64) ? "64" : ""));
+	snprintf(buffer, sizeof (buffer), "%s%s%s%s",
+		 ((resource->flags & IORESOURCE_READONLY) ? "ro" : ""),
+		 ((resource->flags & IORESOURCE_PREFETCH) ? "pref" : ""),
+		 ((resource->flags == 0) ? "unused" :
+		  (resource->flags & IORESOURCE_IO) ? "io" :
+		  (resource->flags & IORESOURCE_DRQ) ? "drq" :
+		  (resource->flags & IORESOURCE_IRQ) ? "irq" :
+		  (resource->flags & IORESOURCE_MEM) ? "mem" : "??????"),
+		 ((resource->flags & IORESOURCE_PCI64) ? "64" : ""));
 	return buffer;
 }
 
@@ -618,10 +623,12 @@ void report_resource_stored(device_t dev, struct resource *resource,
 
 	if (resource->flags & IORESOURCE_PCI_BRIDGE) {
 #if CONFIG_PCI_BUS_SEGN_BITS
-		sprintf(buf, "bus %04x:%02x ", dev->bus->secondary >> 8,
+		snprintf(buf, sizeof (buf),
+			 "bus %04x:%02x ", dev->bus->secondary >> 8,
 			dev->link_list->secondary & 0xff);
 #else
-		sprintf(buf, "bus %02x ", dev->link_list->secondary);
+		snprintf(buf, sizeof (buf),
+			 "bus %02x ", dev->link_list->secondary);
 #endif
 	}
 	printk(BIOS_DEBUG, "%s %02lx <- [0x%010llx - 0x%010llx] size 0x%08llx "
@@ -830,10 +837,12 @@ void show_one_resource(int debug_level, struct device *dev,
 /*
 	if (resource->flags & IORESOURCE_BRIDGE) {
 #if CONFIG_PCI_BUS_SEGN_BITS
-		sprintf(buf, "bus %04x:%02x ", dev->bus->secondary >> 8,
-			dev->link[0].secondary & 0xff);
+		snprintf(buf, sizeof (buf), "bus %04x:%02x ",
+		         dev->bus->secondary >> 8,
+			 dev->link[0].secondary & 0xff);
 #else
-		sprintf(buf, "bus %02x ", dev->link[0].secondary);
+		snprintf(buf, sizeof (buf),
+		         "bus %02x ", dev->link[0].secondary);
 #endif
 	}
 */
diff --git a/src/device/oprom/x86emu/debug.c b/src/device/oprom/x86emu/debug.c
index b3f4b6e..e7a111b 100644
--- a/src/device/oprom/x86emu/debug.c
+++ b/src/device/oprom/x86emu/debug.c
@@ -163,15 +163,15 @@ void x86emu_inc_decoded_inst_len (int x)
 
 void x86emu_decode_printf (const char *x)
 {
-    sprintf(M.x86.decoded_buf+M.x86.enc_str_pos,"%s",x);
+    strcpy(M.x86.decoded_buf+M.x86.enc_str_pos,x);
     M.x86.enc_str_pos += strlen(x);
 }
 
 void x86emu_decode_printf2 (const char *x, int y)
 {
     char temp[100];
-    sprintf(temp,x,y);
-    sprintf(M.x86.decoded_buf+M.x86.enc_str_pos,"%s",temp);
+    snprintf(temp, sizeof (temp), x,y);
+    strcpy(M.x86.decoded_buf+M.x86.enc_str_pos,temp);
     M.x86.enc_str_pos += strlen(temp);
 }
 
@@ -186,7 +186,7 @@ static void print_encoded_bytes (u16 s, u16 o)
     int i;
     char buf1[64];
     for (i=0; i< M.x86.enc_pos; i++) {
-        sprintf(buf1+2*i,"%02x", fetch_data_byte_abs(s,o+i));
+	    snprintf(buf1+2*i, 64 - 2 * i, "%02x", fetch_data_byte_abs(s,o+i));
     }
     printf("%-20s ",buf1);
 }
diff --git a/src/ec/lenovo/h8/h8.c b/src/ec/lenovo/h8/h8.c
index 4f9d228..213ede3 100644
--- a/src/ec/lenovo/h8/h8.c
+++ b/src/ec/lenovo/h8/h8.c
@@ -125,14 +125,14 @@ u8 h8_build_id_and_function_spec_version(char *buf, u8 buf_len)
 	for (i = 0; i < 8; i++) {
 		c = ec_read(0xf0 + i);
 		if (c < 0x20 || c > 0x7f) {
-			i = sprintf(str, "*INVALID");
+		  i = snprintf(str, sizeof (str), "*INVALID");
 			break;
 		}
 		str[i] = c;
 	}
 
 	/* EC firmware function specification version */
-	i += sprintf(str + i, "-%u.%u", ec_read(0xef), ec_read(0xeb));
+	i += snprintf(str + i, sizeof (str) - i, "-%u.%u", ec_read(0xef), ec_read(0xeb));
 
 	i = MIN(buf_len, i);
 	memcpy(buf, str, i);
diff --git a/src/mainboard/google/link/i915.c b/src/mainboard/google/link/i915.c
index 6481234..a38760b 100644
--- a/src/mainboard/google/link/i915.c
+++ b/src/mainboard/google/link/i915.c
@@ -78,7 +78,7 @@ const u32 link_edid_data[] = {
 static char *regname(unsigned long addr)
 {
 	static char name[16];
-	sprintf(name, "0x%lx", addr);
+	snprintf(name, sizeof (name), "0x%lx", addr);
 	return name;
 }
 
diff --git a/src/mainboard/google/stout/i915.c b/src/mainboard/google/stout/i915.c
index 89a8594..8085de6 100644
--- a/src/mainboard/google/stout/i915.c
+++ b/src/mainboard/google/stout/i915.c
@@ -88,7 +88,7 @@ setgtt(int start, int end, unsigned long base, int inc)
 static char *regname(unsigned long addr)
 {
 	static char name[16];
-	sprintf(name, "0x%lx", addr);
+	snprintf(name, sizeof (name), "0x%lx", addr);
 	return name;
 }
 
diff --git a/src/mainboard/intel/wtm2/i915.c b/src/mainboard/intel/wtm2/i915.c
index df88138..b664c35 100644
--- a/src/mainboard/intel/wtm2/i915.c
+++ b/src/mainboard/intel/wtm2/i915.c
@@ -88,7 +88,7 @@ static int ioread = 0, iowrite = 0;
 static char *regname(unsigned long addr)
 {
 	static char name[16];
-	sprintf(name, "0x%lx", addr);
+	snprintf(name, sizeof (name), "0x%lx", addr);
 	return name;
 }
 
diff --git a/src/mainboard/lenovo/x60/i915.c b/src/mainboard/lenovo/x60/i915.c
index 0d32672..ea01ef1 100644
--- a/src/mainboard/lenovo/x60/i915.c
+++ b/src/mainboard/lenovo/x60/i915.c
@@ -80,7 +80,7 @@ const u32 x60_edid_data[] = {
 static char *regname(unsigned long addr)
 {
 	static char name[16];
-	sprintf(name, "0x%lx", addr);
+	snprintf(name, sizeof (name), "0x%lx", addr);
 	return name;
 }
 
diff --git a/src/northbridge/amd/agesa/family10/northbridge.c b/src/northbridge/amd/agesa/family10/northbridge.c
index 1c7e547..60f3951 100644
--- a/src/northbridge/amd/agesa/family10/northbridge.c
+++ b/src/northbridge/amd/agesa/family10/northbridge.c
@@ -657,7 +657,7 @@ static void amdfam10_set_resource(device_t dev, struct resource *resource,
 		store_conf_mmio_addr(nodeid, link_num, reg, (resource->index >>24), rbase>>8, rend>>8);
 	}
 	resource->flags |= IORESOURCE_STORED;
-	sprintf(buf, " <node %x link %x>",
+	snprintf(buf, sizeof (buf), " <node %x link %x>",
 			nodeid, link_num);
 	report_resource_stored(dev, resource, buf);
 }
diff --git a/src/northbridge/amd/agesa/family12/northbridge.c b/src/northbridge/amd/agesa/family12/northbridge.c
index e6be598..9757d96 100644
--- a/src/northbridge/amd/agesa/family12/northbridge.c
+++ b/src/northbridge/amd/agesa/family12/northbridge.c
@@ -402,7 +402,7 @@ static void set_resource(device_t dev, struct resource *resource,
         set_mmio_addr_reg(nodeid, link_num, reg, (resource->index >>24), rbase>>8, rend>>8, 1) ;// [39:8]
     }
     resource->flags |= IORESOURCE_STORED;
-    sprintf(buf, " <node %x link %x>",
+    snprintf(buf, sizeof (buf), " <node %x link %x>",
         nodeid, link_num);
     report_resource_stored(dev, resource, buf);
     printk(BIOS_DEBUG, "Fam12h - northbridge.c - %s - End.\n",__func__);
diff --git a/src/northbridge/amd/agesa/family14/northbridge.c b/src/northbridge/amd/agesa/family14/northbridge.c
index 7cc812b..847e6dd 100644
--- a/src/northbridge/amd/agesa/family14/northbridge.c
+++ b/src/northbridge/amd/agesa/family14/northbridge.c
@@ -402,7 +402,7 @@ static void set_resource(device_t dev, struct resource *resource, u32 nodeid)
 				rbase >> 8, rend >> 8, 1);	// [39:8]
 	}
 	resource->flags |= IORESOURCE_STORED;
-	sprintf(buf, " <node %x link %x>", nodeid, link_num);
+	snprintf(buf, sizeof (buf), " <node %x link %x>", nodeid, link_num);
 	report_resource_stored(dev, resource, buf);
 }
 
diff --git a/src/northbridge/amd/agesa/family15/northbridge.c b/src/northbridge/amd/agesa/family15/northbridge.c
index a8cc1cc..aa9ec56 100644
--- a/src/northbridge/amd/agesa/family15/northbridge.c
+++ b/src/northbridge/amd/agesa/family15/northbridge.c
@@ -400,7 +400,7 @@ static void set_resource(device_t dev, struct resource *resource, u32 nodeid)
 		set_mmio_addr_reg(nodeid, link_num, reg, (resource->index >>24), rbase>>8, rend>>8, node_nums) ;// [39:8]
 	}
 	resource->flags |= IORESOURCE_STORED;
-	sprintf(buf, " <node %x link %x>",
+	snprintf(buf, sizeof (buf), " <node %x link %x>",
 			nodeid, link_num);
 	report_resource_stored(dev, resource, buf);
 }
diff --git a/src/northbridge/amd/agesa/family15tn/northbridge.c b/src/northbridge/amd/agesa/family15tn/northbridge.c
index b55d52b..2b5549d 100644
--- a/src/northbridge/amd/agesa/family15tn/northbridge.c
+++ b/src/northbridge/amd/agesa/family15tn/northbridge.c
@@ -393,7 +393,7 @@ static void set_resource(device_t dev, struct resource *resource, u32 nodeid)
 		set_mmio_addr_reg(nodeid, link_num, reg, (resource->index >>24), rbase>>8, rend>>8, node_nums) ;// [39:8]
 	}
 	resource->flags |= IORESOURCE_STORED;
-	sprintf(buf, " <node %x link %x>",
+	snprintf(buf, sizeof (buf), " <node %x link %x>",
 			nodeid, link_num);
 	report_resource_stored(dev, resource, buf);
 }
diff --git a/src/northbridge/amd/agesa/family16kb/northbridge.c b/src/northbridge/amd/agesa/family16kb/northbridge.c
index b8bba50..6feabf4 100644
--- a/src/northbridge/amd/agesa/family16kb/northbridge.c
+++ b/src/northbridge/amd/agesa/family16kb/northbridge.c
@@ -380,7 +380,7 @@ static void set_resource(device_t dev, struct resource *resource, u32 nodeid)
 		set_mmio_addr_reg(nodeid, link_num, reg, (resource->index >>24), rbase>>8, rend>>8, node_nums) ;// [39:8]
 	}
 	resource->flags |= IORESOURCE_STORED;
-	sprintf(buf, " <node %x link %x>",
+	snprintf(buf, sizeof (buf), " <node %x link %x>",
 			nodeid, link_num);
 	report_resource_stored(dev, resource, buf);
 }
diff --git a/src/northbridge/amd/amdfam10/acpi.c b/src/northbridge/amd/amdfam10/acpi.c
index ee842d5..abba0ce 100644
--- a/src/northbridge/amd/amdfam10/acpi.c
+++ b/src/northbridge/amd/amdfam10/acpi.c
@@ -313,7 +313,7 @@ static void update_sspr(void *sspr, u32 nodeid, u32 cpuindex)
 	CONTROL = sspr + 0x8d;
 	STATUS = sspr + 0x8f;
 
-	sprintf((char*)CPU, "%02x", (char)cpuindex);
+	snprintf((char*)CPU, 3, "%02x", (char)cpuindex);
 	*CPUIN = (u8) cpuindex;
 
 	for(i=0;i<sysconf.p_state_num;i++) {
diff --git a/src/northbridge/amd/amdfam10/northbridge.c b/src/northbridge/amd/amdfam10/northbridge.c
index 6bcab41..26e040f 100644
--- a/src/northbridge/amd/amdfam10/northbridge.c
+++ b/src/northbridge/amd/amdfam10/northbridge.c
@@ -541,8 +541,8 @@ static void amdfam10_set_resource(device_t dev, struct resource *resource,
 		store_conf_mmio_addr(nodeid, link_num, reg, (resource->index >>24), rbase>>8, rend>>8);
 	}
 	resource->flags |= IORESOURCE_STORED;
-	sprintf(buf, " <node %x link %x>",
-		nodeid, link_num);
+	snprintf(buf, sizeof (buf), " <node %x link %x>",
+		 nodeid, link_num);
 	report_resource_stored(dev, resource, buf);
 }
 
diff --git a/src/northbridge/amd/amdk8/northbridge.c b/src/northbridge/amd/amdk8/northbridge.c
index 8f0a11b..b0acdf7 100644
--- a/src/northbridge/amd/amdk8/northbridge.c
+++ b/src/northbridge/amd/amdk8/northbridge.c
@@ -479,8 +479,8 @@ static void amdk8_set_resource(device_t dev, struct resource *resource, unsigned
 		f1_write_config32(reg, base);
 	}
 	resource->flags |= IORESOURCE_STORED;
-	sprintf(buf, " <node %x link %x>",
-		nodeid, link_num);
+	snprintf(buf, sizeof (buf), " <node %x link %x>",
+		 nodeid, link_num);
 	report_resource_stored(dev, resource, buf);
 }
 
diff --git a/src/southbridge/intel/lynxpoint/acpi.c b/src/southbridge/intel/lynxpoint/acpi.c
index b619e6e..ef1c9d8 100644
--- a/src/southbridge/intel/lynxpoint/acpi.c
+++ b/src/southbridge/intel/lynxpoint/acpi.c
@@ -60,7 +60,7 @@ void acpi_create_intel_hpet(acpi_hpet_t * hpet)
 static int acpi_create_serialio_ssdt_entry(int id, global_nvs_t *gnvs)
 {
 	char sio_name[5] = {};
-	sprintf(sio_name, "S%1uEN", id);
+	snprintf(sio_name, sizeof (sio_name), "S%1uEN", id);
 	return acpigen_write_name_byte(sio_name, gnvs->s0b[id] ? 1 : 0);
 }
 



More information about the coreboot-gerrit mailing list