[coreboot-gerrit] [S] Change in coreboot[master]: arch/x86/smbios: Avoid buffer overflows

[Utkarsh Verma (Code Review)]

Utkarsh Verma has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/74129 )

Change subject: arch/x86/smbios: Avoid buffer overflows
......................................................................

arch/x86/smbios: Avoid buffer overflows

The format specifiers for numbers in snprintf() calls are updated to be
more specific to the data types used, making things more obvious for static
analysers.

This was found by coverity scan:
https://scan6.scan.coverity.com/reports.htm#v55284/p10744
CID: 1487449
CID: 1487312
CID: 1487457

Found by: Coverity Scan
Signed-off-by: Utkarsh Verma

Change-Id: I4391f8b44a7e0020a1f01cf0ed6ba75591c0e548
---
M src/arch/x86/smbios.c
M src/arch/x86/smbios_defaults.c
2 files changed, 31 insertions(+), 9 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/29/74129/1

diff --git a/src/arch/x86/smbios.c b/src/arch/x86/smbios.c
index 188fb42..c49b62c 100644
--- a/src/arch/x86/smbios.c
+++ b/src/arch/x86/smbios.c
@@ -145,9 +145,9 @@
    if (manufacturer) {
    	t->manufacturer = smbios_add_string(t->eos, manufacturer);
    } else {
-		char string_buffer[256];
+		char string_buffer[15];

-		snprintf(string_buffer, sizeof(string_buffer), "Unknown (%x)", mod_id);
+		snprintf(string_buffer, sizeof(string_buffer), "Unknown (%hx)", mod_id);
    	t->manufacturer = smbios_add_string(t->eos, string_buffer);
    }
 }
@@ -549,7 +549,7 @@
    unsigned int cpu_voltage;
    struct cpuid_result res;
    uint16_t characteristics = 0;
-	static unsigned int cnt = 0;
+	static unsigned short cnt = 0;
    char buf[8];

/* Provide sane defaults even for CPU without CPUID */
@@ -562,7 +562,7 @@
    struct smbios_type4 *t = smbios_carve_table(*current, SMBIOS_PROCESSOR_INFORMATION,
    					    sizeof(*t), handle);

-	snprintf(buf, sizeof(buf), "CPU%d", cnt++);
+	snprintf(buf, sizeof(buf), "CPU%hhu", cnt++);
    t->socket_designation = smbios_add_string(t->eos, buf);

t->processor_id[0] = res.eax;
diff --git a/src/arch/x86/smbios_defaults.c b/src/arch/x86/smbios_defaults.c
index 8b62ebb..a5fcf78 100644
--- a/src/arch/x86/smbios_defaults.c
+++ b/src/arch/x86/smbios_defaults.c
@@ -8,21 +8,21 @@
 /* this function will fill the corresponding locator */
 __weak void smbios_fill_dimm_locator(const struct dimm_info *dimm, struct smbios_type17 *t)
 {
-	char locator[40];
+	char locator[21];

-	snprintf(locator, sizeof(locator), "Channel-%d-DIMM-%d",
+	snprintf(locator, sizeof(locator), "Channel-%hhu-DIMM-%hhu",
    	dimm->channel_num, dimm->dimm_num);
    t->device_locator = smbios_add_string(t->eos, locator);

-	snprintf(locator, sizeof(locator), "BANK %d", dimm->bank_locator);
+	snprintf(locator, sizeof(locator), "BANK %hhu", dimm->bank_locator);
    t->bank_locator = smbios_add_string(t->eos, locator);
 }

__weak void smbios_fill_dimm_asset_tag(const struct dimm_info *dimm, struct smbios_type17 *t)
 {
-	char buf[40];
+	char buf[30];

-	snprintf(buf, sizeof(buf), "Channel-%d-DIMM-%d-AssetTag",
+	snprintf(buf, sizeof(buf), "Channel-%hhu-DIMM-%hhu-AssetTag",
    	dimm->channel_num, dimm->dimm_num);
    t->asset_tag = smbios_add_string(t->eos, buf);
 }