[L] Change in coreboot[master]: src/arc/x86/smbios.c: Fix coverity issues

[Utkarsh Verma (Code Review)]

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

Change subject: src/arc/x86/smbios.c: Fix coverity issues
......................................................................

src/arc/x86/smbios.c: Fix coverity issues

Coverity scan raised some security issues with memory management for the
smbios sources. This commit serves to address them.

Closes #431

Change-Id: Ibbf93fd113791b6faa1e5128da8e692f7f00ac73
Signed-off-by: Utkarsh Verma 
---
M src/arch/x86/smbios.c
M src/arch/x86/smbios_defaults.c
2 files changed, 166 insertions(+), 191 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/28/74128/1

diff --git a/src/arch/x86/smbios.c b/src/arch/x86/smbios.c
index 188fb42..9b4a422 100644
--- a/src/arch/x86/smbios.c
+++ b/src/arch/x86/smbios.c
@@ -20,12 +20,11 @@
 #include <drivers/vpd/vpd.h>
 #include <stdlib.h>

-#define update_max(len, max_len, stmt)		\
-	do {					\
-		int tmp = stmt;			\
-						\
-		max_len = MAX(max_len, tmp);	\
-		len += tmp;			\
+#define update_max(len, max_len, stmt)                                                         \
+	do {                                                                                   \
+		int tmp = stmt;                                                                \
+		max_len = MAX(max_len, tmp);                                                   \
+		len += tmp;                                                                    \
    } while (0)

static u8 smbios_checksum(u8 *p, u32 length)
@@ -48,21 +47,18 @@
    if (*str == '\0')
    	return 0;

-	for (;;) {
-		if (!*p) {
-			strcpy(p, str);
-			p += strlen(str);
-			*p++ = '\0';
-			*p++ = '\0';
-			return i;
-		}
-
+	while (*p) {
    	if (!strcmp(p, str))
    		return i;

-		p += strlen(p)+1;
+		p += strlen(p) + 1;
    	i++;
    }
+
+	strcpy(p, str);
+	p += strlen(str);
+	*(p + 1) = '\0'; // Indicate the end of set with an additional null byte
+	return i;
 }

int smbios_string_table_len(u8 *start)
@@ -145,9 +141,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);
    }
 }
@@ -160,10 +156,10 @@
    	return;

for (char *p = buffer + len - 1; p >= buffer; --p) {
-		if (*p == ' ')
-			*p = 0;
-		else
+		if (*p != ' ')
    		break;
+
+		*p = 0;
    }
 }

@@ -174,7 +170,7 @@
    size_t i, len;
    char trimmed_part_number[DIMM_INFO_PART_NUMBER_SIZE];

-	strncpy(trimmed_part_number, part_number, sizeof(trimmed_part_number));
+	strncpy(trimmed_part_number, part_number, sizeof(trimmed_part_number) - 1);
    trimmed_part_number[sizeof(trimmed_part_number) - 1] = '\0';

/*
@@ -212,22 +208,20 @@
    				   struct smbios_type17 *t)
 {
    char serial[9];
-
-	snprintf(serial, sizeof(serial), "%02hhx%02hhx%02hhx%02hhx",
-		 dimm->serial[0], dimm->serial[1], dimm->serial[2], dimm->serial[3]);
+	snprintf(serial, sizeof(serial), "%02hhx%02hhx%02hhx%02hhx", dimm->serial[0],
+		 dimm->serial[1], dimm->serial[2], dimm->serial[3]);

t->serial_number = smbios_add_string(t->eos, serial);
 }

-static int create_smbios_type17_for_dimm(struct dimm_info *dimm,
-					 unsigned long *current, int *handle,
-					 int type16_handle)
+static int create_smbios_type17_for_dimm(struct dimm_info *dimm, unsigned long *current,
+					 int *handle, int type16_handle)
 {
    struct spd_info info;
    get_spd_info(dimm->ddr_type, dimm->mod_type, &info);

-	struct smbios_type17 *t = smbios_carve_table(*current, SMBIOS_MEMORY_DEVICE,
-						     sizeof(*t), *handle);
+	struct smbios_type17 *t =
+		smbios_carve_table(*current, SMBIOS_MEMORY_DEVICE, sizeof(*t), *handle);

t->memory_type = dimm->ddr_type;
    if (dimm->configured_speed_mts != 0)
@@ -276,21 +270,20 @@
    return smbios_full_table_len(&t->header, t->eos);
 }

-static int create_smbios_type17_for_empty_slot(struct dimm_info *dimm,
-					       unsigned long *current, int *handle,
-					       int type16_handle)
+static int create_smbios_type17_for_empty_slot(struct dimm_info *dimm, unsigned long *current,
+					       int *handle, int type16_handle)
 {
-	struct smbios_type17 *t = smbios_carve_table(*current, SMBIOS_MEMORY_DEVICE,
-						     sizeof(*t), *handle);
+	struct smbios_type17 *t =
+		smbios_carve_table(*current, SMBIOS_MEMORY_DEVICE, sizeof(*t), *handle);
    t->phys_memory_array_handle = type16_handle;
    /* no handle for error information */
    t->memory_error_information_handle = 0xfffe;
-	t->total_width = 0xffff; /* Unknown */
-	t->data_width = 0xffff; /* Unknown */
-	t->form_factor = 0x2; /* Unknown */
+	t->total_width = 0xffff;	   /* Unknown */
+	t->data_width = 0xffff;		   /* Unknown */
+	t->form_factor = 0x2;		   /* Unknown */
    smbios_fill_dimm_locator(dimm, t); /* Device and Bank */
-	t->memory_type = 0x2; /* Unknown */
-	t->type_detail = 0x2; /* Unknown */
+	t->memory_type = 0x2;		   /* Unknown */
+	t->type_detail = 0x2;		   /* Unknown */

*handle += 1;
    return smbios_full_table_len(&t->header, t->eos);
@@ -324,8 +317,7 @@
 {
    const char *s;

-#define SPACES \
-	"                                                                  "
+#define SPACES "                                                                  "

if (CONFIG(CHROMEOS))
    	return SPACES;
@@ -342,19 +334,19 @@

if (strlen(CONFIG_LOCALVERSION) != 0) {
    	printk(BIOS_DEBUG, "BIOS version set to CONFIG_LOCALVERSION: '%s'\n",
-			CONFIG_LOCALVERSION);
+		       CONFIG_LOCALVERSION);
    	return CONFIG_LOCALVERSION;
    }

printk(BIOS_DEBUG, "SMBIOS firmware version is set to coreboot_version: '%s'\n",
-		coreboot_version);
+	       coreboot_version);
    return coreboot_version;
 }

static int smbios_write_type0(unsigned long *current, int handle)
 {
-	struct smbios_type0 *t = smbios_carve_table(*current, SMBIOS_BIOS_INFORMATION,
-						    sizeof(*t), handle);
+	struct smbios_type0 *t =
+		smbios_carve_table(*current, SMBIOS_BIOS_INFORMATION, sizeof(*t), handle);

t->vendor = smbios_add_string(t->eos, "coreboot");
    t->bios_release_date = smbios_add_string(t->eos, coreboot_dmi_date);
@@ -380,10 +372,9 @@

smbios_ec_revision(&t->ec_major_release, &t->ec_minor_release);

-	t->bios_characteristics =
-		BIOS_CHARACTERISTICS_PCI_SUPPORTED |
-		BIOS_CHARACTERISTICS_SELECTABLE_BOOT |
-		BIOS_CHARACTERISTICS_UPGRADEABLE;
+	t->bios_characteristics = BIOS_CHARACTERISTICS_PCI_SUPPORTED
+				  | BIOS_CHARACTERISTICS_SELECTABLE_BOOT
+				  | BIOS_CHARACTERISTICS_UPGRADEABLE;

if (CONFIG(CARDBUS_PLUGIN_SUPPORT))
    	t->bios_characteristics |= BIOS_CHARACTERISTICS_PC_CARD;
@@ -441,7 +432,7 @@

unsigned int __weak smbios_cache_sram_type(void)
 {
-	return  SMBIOS_CACHE_SRAM_TYPE_UNKNOWN;
+	return SMBIOS_CACHE_SRAM_TYPE_UNKNOWN;
 }

unsigned int __weak smbios_cache_conf_operation_mode(u8 level)
@@ -484,8 +475,8 @@

static int smbios_write_type1(unsigned long *current, int handle)
 {
-	struct smbios_type1 *t = smbios_carve_table(*current, SMBIOS_SYSTEM_INFORMATION,
-						    sizeof(*t), handle);
+	struct smbios_type1 *t =
+		smbios_carve_table(*current, SMBIOS_SYSTEM_INFORMATION, sizeof(*t), handle);

t->manufacturer = smbios_add_string(t->eos, smbios_system_manufacturer());
    t->product_name = smbios_add_string(t->eos, smbios_system_product_name());
@@ -504,8 +495,8 @@

static int smbios_write_type2(unsigned long *current, int handle, const int chassis_handle)
 {
-	struct smbios_type2 *t = smbios_carve_table(*current, SMBIOS_BOARD_INFORMATION,
-						    sizeof(*t), handle);
+	struct smbios_type2 *t =
+		smbios_carve_table(*current, SMBIOS_BOARD_INFORMATION, sizeof(*t), handle);

t->manufacturer = smbios_add_string(t->eos, smbios_mainboard_manufacturer());
    t->product_name = smbios_add_string(t->eos, smbios_mainboard_product_name());
@@ -513,8 +504,8 @@
    t->version = smbios_add_string(t->eos, smbios_mainboard_version());
    t->asset_tag = smbios_add_string(t->eos, smbios_mainboard_asset_tag());
    t->feature_flags = smbios_mainboard_feature_flags();
-	t->location_in_chassis = smbios_add_string(t->eos,
-		smbios_mainboard_location_in_chassis());
+	t->location_in_chassis =
+		smbios_add_string(t->eos, smbios_mainboard_location_in_chassis());
    t->board_type = smbios_mainboard_board_type();
    t->chassis_handle = chassis_handle;
    const int len = smbios_full_table_len(&t->header, t->eos);
@@ -524,8 +515,8 @@

static int smbios_write_type3(unsigned long *current, int handle)
 {
-	struct smbios_type3 *t = smbios_carve_table(*current, SMBIOS_SYSTEM_ENCLOSURE,
-						    sizeof(*t), handle);
+	struct smbios_type3 *t =
+		smbios_carve_table(*current, SMBIOS_SYSTEM_ENCLOSURE, sizeof(*t), handle);

t->manufacturer = smbios_add_string(t->eos, smbios_system_manufacturer());
    t->bootup_state = SMBIOS_STATE_SAFE;
@@ -549,7 +540,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 */
@@ -559,10 +550,10 @@
    if (cpu_have_cpuid())
    	res = cpuid(1);

-	struct smbios_type4 *t = smbios_carve_table(*current, SMBIOS_PROCESSOR_INFORMATION,
-						    sizeof(*t), handle);
+	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;
@@ -648,26 +639,23 @@
  * Fill in some fields with constant values, as gathering the information
  * from CPUID is impossible.
  */
-static int smbios_write_type7(unsigned long *current,
-			      const int handle,
-			      const u8 level,
+static int smbios_write_type7(unsigned long *current, const int handle, const u8 level,
    		      const u8 sram_type,
    		      const enum smbios_cache_associativity associativity,
-			      const enum smbios_cache_type type,
-			      const size_t max_cache_size,
+			      const enum smbios_cache_type type, const size_t max_cache_size,
    		      const size_t cache_size)
 {
    char buf[8];

-	struct smbios_type7 *t = smbios_carve_table(*current, SMBIOS_CACHE_INFORMATION,
-						    sizeof(*t), handle);
+	struct smbios_type7 *t =
+		smbios_carve_table(*current, SMBIOS_CACHE_INFORMATION, sizeof(*t), handle);

-	snprintf(buf, sizeof(buf), "CACHE%x", level);
+	snprintf(buf, sizeof(buf), "CACHE%hhx", level);
    t->socket_designation = smbios_add_string(t->eos, buf);

-	t->cache_configuration = SMBIOS_CACHE_CONF_LEVEL(level) |
-		SMBIOS_CACHE_CONF_LOCATION(0) | /* Internal */
-		SMBIOS_CACHE_CONF_ENABLED(1) | /* Enabled */
+	t->cache_configuration =
+		SMBIOS_CACHE_CONF_LEVEL(level) | SMBIOS_CACHE_CONF_LOCATION(0) | /* Internal */
+		SMBIOS_CACHE_CONF_ENABLED(1) |					 /* Enabled */
    	SMBIOS_CACHE_CONF_OPERATION_MODE(smbios_cache_conf_operation_mode(level));

if (max_cache_size < (SMBIOS_CACHE_SIZE_MASK * KiB)) {
@@ -758,10 +746,8 @@
  * @param max_struct_size Pointer to maximum struct size
  * @param type4 Pointer to SMBIOS type 4 structure
  */
-static int smbios_write_type7_cache_parameters(unsigned long *current,
-					       int *handle,
-					       int *max_struct_size,
-					       struct smbios_type4 *type4)
+static int smbios_write_type7_cache_parameters(unsigned long *current, int *handle,
+					       int *max_struct_size, struct smbios_type4 *type4)
 {
    unsigned int cnt = CACHE_L1D;
    int len = 0;
@@ -771,8 +757,9 @@

enum cpu_type dcache_cpuid = cpu_check_deterministic_cache_cpuid_supported();
    if (dcache_cpuid == CPUID_TYPE_INVALID || dcache_cpuid == CPUID_COMMAND_UNSUPPORTED) {
-		printk(BIOS_DEBUG, "SMBIOS: Unknown CPU or CPU doesn't support Deterministic "
-					"Cache CPUID leaf\n");
+		printk(BIOS_DEBUG,
+		       "SMBIOS: Unknown CPU or CPU doesn't support Deterministic "
+		       "Cache CPUID leaf\n");
    	return len;
    }

@@ -815,9 +802,9 @@

const int h = (*handle)++;

-		update_max(len, *max_struct_size, smbios_write_type7(current, h,
-			   level, smbios_cache_sram_type(), associativity,
-			   type, cache_size, cache_size));
+		update_max(len, *max_struct_size,
+			   smbios_write_type7(current, h, level, smbios_cache_sram_type(),
+					      associativity, type, cache_size, cache_size));

if (type4) {
    		switch (level) {
@@ -837,16 +824,14 @@
    return len;
 }

-int smbios_write_type8(unsigned long *current, int *handle,
-				const struct port_information *port,
-				size_t num_ports)
+int smbios_write_type8(unsigned long *current, int *handle, const struct port_information *port,
+		       size_t num_ports)
 {
    unsigned int totallen = 0, i;

for (i = 0; i < num_ports; i++, port++) {
-		struct smbios_type8 *t = smbios_carve_table(*current,
-							    SMBIOS_PORT_CONNECTOR_INFORMATION,
-							    sizeof(*t), *handle);
+		struct smbios_type8 *t = smbios_carve_table(
+			*current, SMBIOS_PORT_CONNECTOR_INFORMATION, sizeof(*t), *handle);
    	t->internal_reference_designator =
    		smbios_add_string(t->eos, port->internal_reference_designator);
    	t->internal_connector_type = port->internal_connector_type;
@@ -862,15 +847,14 @@
    return totallen;
 }

-int smbios_write_type9(unsigned long *current, int *handle,
-			const char *name, const enum misc_slot_type type,
-			const enum slot_data_bus_bandwidth bandwidth,
-			const enum misc_slot_usage usage,
-			const enum misc_slot_length length,
-			const u16 id, u8 slot_char1, u8 slot_char2, u8 bus, u8 dev_func)
+int smbios_write_type9(unsigned long *current, int *handle, const char *name,
+		       const enum misc_slot_type type,
+		       const enum slot_data_bus_bandwidth bandwidth,
+		       const enum misc_slot_usage usage, const enum misc_slot_length length,
+		       const u16 id, u8 slot_char1, u8 slot_char2, u8 bus, u8 dev_func)
 {
-	struct smbios_type9 *t = smbios_carve_table(*current, SMBIOS_SYSTEM_SLOTS,
-						    sizeof(*t), *handle);
+	struct smbios_type9 *t =
+		smbios_carve_table(*current, SMBIOS_SYSTEM_SLOTS, sizeof(*t), *handle);

t->slot_designation = smbios_add_string(t->eos, name ? name : "SLOT");
    t->slot_type = type;
@@ -895,8 +879,8 @@
 static int smbios_write_type11(unsigned long *current, int *handle)
 {
    struct device *dev;
-	struct smbios_type11 *t = smbios_carve_table(*current, SMBIOS_OEM_STRINGS,
-						     sizeof(*t), *handle);
+	struct smbios_type11 *t =
+		smbios_carve_table(*current, SMBIOS_OEM_STRINGS, sizeof(*t), *handle);

for (dev = all_devices; dev; dev = dev->next) {
    	if (dev->ops && dev->ops->get_smbios_strings)
@@ -922,7 +906,7 @@
    struct memory_info *meminfo;
    meminfo = cbmem_find(CBMEM_ID_MEMINFO);
    if (meminfo == NULL)
-		return 0;	/* can't find mem info in cbmem */
+		return 0; /* can't find mem info in cbmem */

printk(BIOS_INFO, "Create SMBIOS type 16\n");

@@ -936,8 +920,8 @@
    	}
    }

-	struct smbios_type16 *t = smbios_carve_table(*current, SMBIOS_PHYS_MEMORY_ARRAY,
-						     sizeof(*t), *handle);
+	struct smbios_type16 *t =
+		smbios_carve_table(*current, SMBIOS_PHYS_MEMORY_ARRAY, sizeof(*t), *handle);

t->location = MEMORY_ARRAY_LOCATION_SYSTEM_BOARD;
    t->use = MEMORY_ARRAY_USE_SYSTEM;
@@ -965,10 +949,9 @@
    int totallen = 0;
    int i;

-	struct memory_info *meminfo;
-	meminfo = cbmem_find(CBMEM_ID_MEMINFO);
+	struct memory_info *meminfo = cbmem_find(CBMEM_ID_MEMINFO);
    if (meminfo == NULL)
-		return 0;	/* can't find mem info in cbmem */
+		return 0; /* can't find mem info in cbmem */

printk(BIOS_INFO, "Create SMBIOS type 17\n");
    for (i = 0; i < meminfo->dimm_cnt && i < ARRAY_SIZE(meminfo->dimm); i++) {
@@ -998,11 +981,10 @@
    struct memory_info *meminfo;
    meminfo = cbmem_find(CBMEM_ID_MEMINFO);
    if (meminfo == NULL)
-		return 0;	/* can't find mem info in cbmem */
+		return 0; /* can't find mem info in cbmem */

-	struct smbios_type19 *t = smbios_carve_table(*current,
-						     SMBIOS_MEMORY_ARRAY_MAPPED_ADDRESS,
-						     sizeof(*t), *handle);
+	struct smbios_type19 *t = smbios_carve_table(
+		*current, SMBIOS_MEMORY_ARRAY_MAPPED_ADDRESS, sizeof(*t), *handle);

t->memory_array_handle = type16;

@@ -1015,8 +997,8 @@
    t->extended_ending_address *= MiB;

/* Check if it fits into regular address */
-	if (t->extended_ending_address >= KiB &&
-	    t->extended_ending_address < 0x40000000000ULL) {
+	if (t->extended_ending_address >= KiB
+	    && t->extended_ending_address < 0x40000000000ULL) {
    	/*
    	 * FIXME: The starting address is SoC specific, but SMBIOS tables are only
    	 * exported on x86 where it's always 0.
@@ -1040,10 +1022,10 @@
 }

static int smbios_write_type20_table(unsigned long *current, int *handle, u32 addr_start,
-		u32 addr_end, int type17_handle, int type19_handle)
+				     u32 addr_end, int type17_handle, int type19_handle)
 {
-	struct smbios_type20 *t = smbios_carve_table(*current, SMBIOS_MEMORY_DEVICE_MAPPED_ADDRESS,
-						     sizeof(*t), *handle);
+	struct smbios_type20 *t = smbios_carve_table(
+		*current, SMBIOS_MEMORY_DEVICE_MAPPED_ADDRESS, sizeof(*t), *handle);

t->memory_device_handle = type17_handle;
    t->memory_array_mapped_address_handle = type19_handle;
@@ -1059,45 +1041,38 @@
    return len;
 }

-static int smbios_write_type20(unsigned long *current, int *handle,
-		int type17_handle, int type19_handle)
+static int smbios_write_type20(unsigned long *current, int *handle, int type17_handle,
+			       int type19_handle)
 {
    u32 start_addr = 0;
    int totallen = 0;
    int i;

-	struct memory_info *meminfo;
-	meminfo = cbmem_find(CBMEM_ID_MEMINFO);
+	struct memory_info *meminfo = cbmem_find(CBMEM_ID_MEMINFO);
    if (meminfo == NULL)
-		return 0;	/* can't find mem info in cbmem */
+		return 0; /* can't find mem info in cbmem */

printk(BIOS_INFO, "Create SMBIOS type 20\n");
    for (i = 0; i < meminfo->dimm_cnt && i < ARRAY_SIZE(meminfo->dimm); i++) {
-		struct dimm_info *dimm;
-		dimm = &meminfo->dimm[i];
+		struct dimm_info *dimm = &meminfo->dimm[i];
    	if (dimm->dimm_size == 0)
    		continue;

u32 end_addr = start_addr + (dimm->dimm_size << 10) - 1;
    	totallen += smbios_write_type20_table(current, handle, start_addr, end_addr,
-				type17_handle, type19_handle);
+						      type17_handle, type19_handle);
    	start_addr = end_addr + 1;
    }
    return totallen;
 }

-int smbios_write_type28(unsigned long *current, int *handle,
-			const char *name,
+int smbios_write_type28(unsigned long *current, int *handle, const char *name,
    		const enum smbios_temp_location location,
-			const enum smbios_temp_status status,
-			u16 max_value, u16 min_value,
-			u16 resolution, u16 tolerance,
-			u16 accuracy,
-			u32 oem,
-			u16 nominal_value)
+			const enum smbios_temp_status status, u16 max_value, u16 min_value,
+			u16 resolution, u16 tolerance, u16 accuracy, u32 oem, u16 nominal_value)
 {
-	struct smbios_type28 *t = smbios_carve_table(*current, SMBIOS_TEMPERATURE_PROBE,
-						     sizeof(*t), *handle);
+	struct smbios_type28 *t =
+		smbios_carve_table(*current, SMBIOS_TEMPERATURE_PROBE, sizeof(*t), *handle);

t->description = smbios_add_string(t->eos, name ? name : "Temperature");
    t->location_and_status = location | (status << 5);
@@ -1126,10 +1101,9 @@
 }

int smbios_write_type38(unsigned long *current, int *handle,
-			const enum smbios_bmc_interface_type interface_type,
-			const u8 ipmi_rev, const u8 i2c_addr, const u8 nv_addr,
-			const u64 base_addr, const u8 base_modifier,
-			const u8 irq)
+			const enum smbios_bmc_interface_type interface_type, const u8 ipmi_rev,
+			const u8 i2c_addr, const u8 nv_addr, const u64 base_addr,
+			const u8 base_modifier, const u8 irq)
 {
    struct smbios_type38 *t = smbios_carve_table(*current, SMBIOS_IPMI_DEVICE_INFORMATION,
    					     sizeof(*t), *handle);
@@ -1148,16 +1122,13 @@
    return len;
 }

-int smbios_write_type39(unsigned long *current, int *handle,
-			u8 unit_group, const char *loc, const char *dev_name,
-			const char *man, const char *serial_num,
-			const char *tag_num, const char *part_num,
-			const char *rev_lvl, u16 max_pow_cap,
-			const struct power_supply_ch *ps_ch)
+int smbios_write_type39(unsigned long *current, int *handle, u8 unit_group, const char *loc,
+			const char *dev_name, const char *man, const char *serial_num,
+			const char *tag_num, const char *part_num, const char *rev_lvl,
+			u16 max_pow_cap, const struct power_supply_ch *ps_ch)
 {
-	struct smbios_type39 *t = smbios_carve_table(*current,
-						SMBIOS_SYSTEM_POWER_SUPPLY,
-						sizeof(*t), *handle);
+	struct smbios_type39 *t =
+		smbios_carve_table(*current, SMBIOS_SYSTEM_POWER_SUPPLY, sizeof(*t), *handle);

uint16_t val = 0;
    uint16_t ps_type, ps_status, vol_switch, ps_unplug, ps_present, hot_rep;
@@ -1197,13 +1168,11 @@
    return len;
 }

-int smbios_write_type41(unsigned long *current, int *handle,
-			const char *name, u8 instance, u16 segment,
-			u8 bus, u8 device, u8 function, u8 device_type)
+int smbios_write_type41(unsigned long *current, int *handle, const char *name, u8 instance,
+			u16 segment, u8 bus, u8 device, u8 function, u8 device_type)
 {
-	struct smbios_type41 *t = smbios_carve_table(*current,
-						SMBIOS_ONBOARD_DEVICES_EXTENDED_INFORMATION,
-						sizeof(*t), *handle);
+	struct smbios_type41 *t = smbios_carve_table(
+		*current, SMBIOS_ONBOARD_DEVICES_EXTENDED_INFORMATION, sizeof(*t), *handle);

t->reference_designation = smbios_add_string(t->eos, name);
    t->device_type = device_type;
@@ -1221,12 +1190,12 @@
 }

int smbios_write_type43(unsigned long *current, int *handle, const u32 vendor_id,
-			const u8 major_spec_ver, const u8 minor_spec_ver,
-			const u32 fw_ver1, const u32 fw_ver2, const char *description,
-			const u64 characteristics, const u32 oem_defined)
+			const u8 major_spec_ver, const u8 minor_spec_ver, const u32 fw_ver1,
+			const u32 fw_ver2, const char *description, const u64 characteristics,
+			const u32 oem_defined)
 {
-	struct smbios_type43 *t = smbios_carve_table(*current, SMBIOS_TPM_DEVICE,
-						     sizeof(*t), *handle);
+	struct smbios_type43 *t =
+		smbios_carve_table(*current, SMBIOS_TPM_DEVICE, sizeof(*t), *handle);

t->vendor_id = vendor_id;
    t->major_spec_ver = major_spec_ver;
@@ -1245,8 +1214,8 @@

static int smbios_write_type127(unsigned long *current, int handle)
 {
-	struct smbios_type127 *t = smbios_carve_table(*current, SMBIOS_END_OF_TABLE,
-						      sizeof(*t), handle);
+	struct smbios_type127 *t =
+		smbios_carve_table(*current, SMBIOS_END_OF_TABLE, sizeof(*t), handle);

const int len = smbios_full_table_len(&t->header, t->eos);
    *current += len;
@@ -1296,8 +1265,8 @@
 #else
    static u8 type41_inst_cnt[SMBIOS_DEVICE_TYPE_COUNT + 1] = {};

-	if (device_type == SMBIOS_DEVICE_TYPE_OTHER ||
-	    device_type == SMBIOS_DEVICE_TYPE_UNKNOWN)
+	if (device_type == SMBIOS_DEVICE_TYPE_OTHER
+	    || device_type == SMBIOS_DEVICE_TYPE_UNKNOWN)
    	return false;

if (device_type > SMBIOS_DEVICE_TYPE_COUNT)
@@ -1335,13 +1304,13 @@
    const char *name = smbios_get_type41_refdes(dev);

return smbios_write_type41(current, handle,
-					name, // name
-					instance_id, // inst
-					0, // segment
-					dev->bus->secondary, //bus
-					PCI_SLOT(dev->path.pci.devfn), // device
-					PCI_FUNC(dev->path.pci.devfn), // func
-					device_type);
+				   name,			  // name
+				   instance_id,			  // inst
+				   0,				  // segment
+				   dev->bus->secondary,		  // bus
+				   PCI_SLOT(dev->path.pci.devfn), // device
+				   PCI_FUNC(dev->path.pci.devfn), // func
+				   device_type);
 }

static int smbios_generate_type9_from_devtree(struct device *dev, int *handle,
@@ -1355,8 +1324,8 @@
    if (dev->path.type != DEVICE_PATH_PCI)
    	return 0;

-	if (!dev->smbios_slot_type && !dev->smbios_slot_data_width &&
-	    !dev->smbios_slot_designation && !dev->smbios_slot_length)
+	if (!dev->smbios_slot_type && !dev->smbios_slot_data_width
+	    && !dev->smbios_slot_designation && !dev->smbios_slot_length)
    	return 0;

if (dev_is_active_bridge(dev))
@@ -1381,16 +1350,8 @@
    else
    	length = SlotLengthUnknown;

-	return smbios_write_type9(current, handle,
-				  dev->smbios_slot_designation,
-				  type,
-				  bandwidth,
-				  usage,
-				  length,
-				  0,
-				  1,
-				  0,
-				  dev->bus->secondary,
+	return smbios_write_type9(current, handle, dev->smbios_slot_designation, type,
+				  bandwidth, usage, length, 0, 1, 0, dev->bus->secondary,
    			  dev->path.pci.devfn);
 }

@@ -1457,8 +1418,7 @@
    len += smbios_write_type7_cache_parameters(&current, &handle, &max_struct_size, type4);
    update_max(len, max_struct_size, smbios_write_type11(&current, &handle));
    if (CONFIG(ELOG))
-		update_max(len, max_struct_size,
-			elog_smbios_write_type15(&current, handle++));
+		update_max(len, max_struct_size, elog_smbios_write_type15(&current, handle++));

const int type16 = handle;
    update_max(len, max_struct_size, smbios_write_type16(&current, &handle));
@@ -1467,11 +1427,11 @@
    const int type19 = handle;
    update_max(len, max_struct_size, smbios_write_type19(&current, &handle, type16));
    update_max(len, max_struct_size,
-			smbios_write_type20(&current, &handle, type17, type19));
+		   smbios_write_type20(&current, &handle, type17, type19));
    update_max(len, max_struct_size, smbios_write_type32(&current, handle++));

-	update_max(len, max_struct_size, smbios_walk_device_tree(all_devices,
-								 &handle, &current));
+	update_max(len, max_struct_size,
+		   smbios_walk_device_tree(all_devices, &handle, &current));

update_max(len, max_struct_size, smbios_write_type127(&current, handle++));

diff --git a/src/arch/x86/smbios_defaults.c b/src/arch/x86/smbios_defaults.c
index 8b62ebb..bf53409 100644
--- a/src/arch/x86/smbios_defaults.c
+++ b/src/arch/x86/smbios_defaults.c
@@ -8,22 +8,22 @@
 /* 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",
-		dimm->channel_num, dimm->dimm_num);
+	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",
-		dimm->channel_num, dimm->dimm_num);
+	snprintf(buf, sizeof(buf), "Channel-%hhu-DIMM-%hhu-AssetTag", dimm->channel_num,
+		 dimm->dimm_num);
    t->asset_tag = smbios_add_string(t->eos, buf);
 }