src: Remove variable length arrays

Variable length arrays were a feature added in C99 that allows the
length of an array to be determined at runtime. Eg.

	int sum(size_t n) {
		int arr[n];
		...
	}

This adds a small amount of runtime overhead, but is also very
dangerous, since it allows use of an unlimited amount of stack memory,
potentially leading to stack overflow. This is only worsened in
coreboot, which often has very little stack space to begin with. Citing
concerns like this, all instances of VLA's were recently removed from the
Linux kernel. In the immortal words of Linus Torvalds [0],

    AND USING VLA'S IS ACTIVELY STUPID! It generates much more code, and
    much _slower_ code (and more fragile code), than just using a fixed
    key size would have done. [...] Anyway, some of these are definitely
    easy to just fix, and using VLA's is actively bad not just for
    security worries, but simply because VLA's are a really horribly bad
    idea in general in the kernel.

This patch follows suit and zaps all VLA's in coreboot. Some of the
existing VLA's are accidental ones, and all but one can be replaced with
small fixed-size buffers. The single tricky exception is in the SPI
controller interface, which will require a rewrite of old drivers
to remove [1].

[0] https://lkml.org/lkml/2018/3/7/621
[1] https://ticket.coreboot.org/issues/217

Change-Id: I7d9d1ddadbf1cee5f695165bbe3f0effb7bd32b9
Signed-off-by: Jacob Garber <jgarber1@ualberta.ca>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/33821
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Patrick Georgi <pgeorgi@google.com>

src/arch/x86/smbios.c

@@ -275,20 +275,18 @@ static void trim_trailing_whitespace(char *buffer, size_t buffer_size)
 static void smbios_fill_dimm_part_number(const char *part_number,
 					 struct smbios_type17 *t)
 {
-	const size_t trimmed_buffer_size = DIMM_INFO_PART_NUMBER_SIZE;
-
 	int invalid;
 	size_t i, len;
-	char trimmed_part_number[trimmed_buffer_size];
+	char trimmed_part_number[DIMM_INFO_PART_NUMBER_SIZE];
 
-	strncpy(trimmed_part_number, part_number, trimmed_buffer_size);
+	strncpy(trimmed_part_number, part_number, sizeof(trimmed_part_number));
-	trimmed_part_number[trimmed_buffer_size - 1] = '\0';
+	trimmed_part_number[sizeof(trimmed_part_number) - 1] = '\0';
 
 	/*
 	 * SPD mandates that unused characters be represented with a ' '.
 	 * We don't want to publish the whitespace in the SMBIOS tables.
 	 */
-	trim_trailing_whitespace(trimmed_part_number, trimmed_buffer_size);
+	trim_trailing_whitespace(trimmed_part_number, sizeof(trimmed_part_number));
 
 	len = strlen(trimmed_part_number);
 
@@ -304,8 +302,7 @@ static void smbios_fill_dimm_part_number(const char *part_number,
 		/* Null String in Part Number will have "None" instead. */
 		t->part_number = smbios_add_string(t->eos, "None");
 	} else if (invalid) {
-		char string_buffer[trimmed_buffer_size +
+		char string_buffer[sizeof(trimmed_part_number) + 10];
-			   10 /* strlen("Invalid ()") */];
 
 		snprintf(string_buffer, sizeof(string_buffer), "Invalid (%s)",
 			 trimmed_part_number);

src/drivers/spi/spi_flash.c

@@ -100,6 +100,7 @@ int spi_flash_cmd(const struct spi_slave *spi, u8 cmd, void *response, size_t le
 #pragma GCC diagnostic push
 #if defined(__GNUC__) && !defined(__clang__)
 #pragma GCC diagnostic ignored "-Wstack-usage="
+#pragma GCC diagnostic ignored "-Wvla"
 #endif
 int spi_flash_cmd_write(const struct spi_slave *spi, const u8 *cmd,
 			size_t cmd_len, const void *data, size_t data_len)

src/northbridge/amd/amdmct/mct_ddr3/mctdqs_d.c

@@ -1252,11 +1252,15 @@ void write_dram_dqs_training_pattern_fam15(struct MCTStatStruc *pMCTstat,
 	stop_dram_dqs_training_pattern_fam15(pMCTstat, pDCTstat, dct, Receiver);
 }
 
+#define LANE_DIFF 1
+
 /* DQS Position Training
  * Algorithm detailed in the Fam15h BKDG Rev. 3.14 section 2.10.5.8.4
  */
 static uint8_t TrainDQSRdWrPos_D_Fam15(struct MCTStatStruc *pMCTstat,
-				struct DCTStatStruc *pDCTstat, uint8_t dct, uint8_t receiver_start, uint8_t receiver_end, uint8_t lane_start, uint8_t lane_end)
+				       struct DCTStatStruc *pDCTstat,
+				       uint8_t dct, uint8_t receiver_start,
+				       uint8_t receiver_end, uint8_t lane_start)
 {
 	uint8_t dimm;
 	uint8_t lane;
@@ -1276,7 +1280,8 @@ static uint8_t TrainDQSRdWrPos_D_Fam15(struct MCTStatStruc *pMCTstat,
 	uint16_t current_read_dqs_delay[MAX_BYTE_LANES];
 	uint16_t current_write_dqs_delay[MAX_BYTE_LANES];
 	uint8_t passing_dqs_delay_found[MAX_BYTE_LANES];
-	uint8_t dqs_results_array[2][(lane_end - lane_start)][32][48];		/* [rank][lane][write step][read step + 16] */
+	/* [rank][lane][write step][read step + 16] */
+	uint8_t dqs_results_array[2][LANE_DIFF][32][48];
 
 	uint8_t last_pos = 0;
 	uint8_t cur_count = 0;
@@ -1286,6 +1291,8 @@ static uint8_t TrainDQSRdWrPos_D_Fam15(struct MCTStatStruc *pMCTstat,
 	uint32_t index_reg = 0x98;
 	uint32_t dev = pDCTstat->dev_dct;
 
+	uint8_t lane_end = lane_start + LANE_DIFF;
+
 	uint8_t lane_count;
 	lane_count = get_available_lane_count(pMCTstat, pDCTstat);
 
@@ -1734,7 +1741,10 @@ static void TrainDQSReceiverEnCyc_D_Fam15(struct MCTStatStruc *pMCTstat,
 					Calc_SetMaxRdLatency_D_Fam15(pMCTstat, pDCTstat, dct, 0);
 
 					/* 2.10.5.8.3 (4 B) */
-					dqs_results_array[current_phy_phase_delay[lane]] = TrainDQSRdWrPos_D_Fam15(pMCTstat, pDCTstat, dct, Receiver, Receiver + 2, lane, lane + 1);
+					dqs_results_array[current_phy_phase_delay[lane]] =
+						TrainDQSRdWrPos_D_Fam15(pMCTstat, pDCTstat, dct,
+									Receiver, Receiver + 2,
+									lane);
 
 					if (dqs_results_array[current_phy_phase_delay[lane]])
 						lane_success_count++;
@@ -1790,7 +1800,9 @@ static void TrainDQSReceiverEnCyc_D_Fam15(struct MCTStatStruc *pMCTstat,
 
 						/* Update hardware registers with final values */
 						write_dqs_receiver_enable_control_registers(current_phy_phase_delay, dev, dct, dimm, index_reg);
-						TrainDQSRdWrPos_D_Fam15(pMCTstat, pDCTstat, dct, Receiver, Receiver + 2, lane, lane + 1);
+						TrainDQSRdWrPos_D_Fam15(pMCTstat, pDCTstat, dct,
+									Receiver, Receiver + 2,
+									lane);
 						break;
 					}
 					prev = dqs_results_array[current_phy_phase_delay[lane]];

src/northbridge/amd/amdmct/mct_ddr3/mhwlc_d.c

@@ -15,6 +15,7 @@
  */
 
 #include <stdint.h>
+#include <assert.h>
 #include <console/console.h>
 #include <northbridge/amd/amdfam10/amdfam10.h>
 
@@ -31,6 +32,8 @@ void procConfig(struct MCTStatStruc *pMCTstat, struct DCTStatStruc *pDCTstat, ui
 void setWLByteDelay(struct DCTStatStruc *pDCTstat, uint8_t dct, u8 ByteLane, u8 dimm, u8 targetAddr, uint8_t pass, uint8_t lane_count);
 void getWLByteDelay(struct DCTStatStruc *pDCTstat, uint8_t dct, u8 ByteLane, u8 dimm, uint8_t pass, uint8_t nibble, uint8_t lane_count);
 
+#define MAX_LANE_COUNT 9
+
 /*-----------------------------------------------------------------------------
  * uint8_t AgesaHwWlPhase1(SPDStruct *SPDData,MCTStruct *MCTData, DCTStruct *DCTData,
  *                  u8 Dimm, u8 Pass)
@@ -185,8 +188,10 @@ uint8_t AgesaHwWlPhase2(struct MCTStatStruc *pMCTstat, struct DCTStatStruc *pDCT
 
 	lane_count = get_available_lane_count(pMCTstat, pDCTstat);
 
+	assert(lane_count <= MAX_LANE_COUNT);
+
 	if (is_fam15h()) {
-		int32_t gross_diff[lane_count];
+		int32_t gross_diff[MAX_LANE_COUNT];
 		int32_t cgd = pDCTData->WLCriticalGrossDelayPrevPass;
 		uint8_t index = (uint8_t)(lane_count * dimm);
 
@@ -274,9 +279,11 @@ uint8_t AgesaHwWlPhase3(struct MCTStatStruc *pMCTstat, struct DCTStatStruc *pDCT
 
 	lane_count = get_available_lane_count(pMCTstat, pDCTstat);
 
+	assert(lane_count <= MAX_LANE_COUNT);
+
 	if (is_fam15h()) {
 		uint32_t dword;
-		int32_t gross_diff[lane_count];
+		int32_t gross_diff[MAX_LANE_COUNT];
 		int32_t cgd = pDCTData->WLCriticalGrossDelayPrevPass;
 		uint8_t index = (uint8_t)(lane_count * dimm);
 
@@ -1005,6 +1012,8 @@ void procConfig(struct MCTStatStruc *pMCTstat, struct DCTStatStruc *pDCTstat, ui
 
 	lane_count = get_available_lane_count(pMCTstat, pDCTstat);
 
+	assert(lane_count <= MAX_LANE_COUNT);
+
 	if (is_fam15h()) {
 		/* MemClkFreq: 0x4: 333MHz; 0x6: 400MHz; 0xa: 533MHz; 0xe: 667MHz; 0x12: 800MHz; 0x16: 933MHz */
 		MemClkFreq = get_Bits(pDCTData, dct, pDCTData->NodeId,
@@ -1168,8 +1177,8 @@ void procConfig(struct MCTStatStruc *pMCTstat, struct DCTStatStruc *pDCTstat, ui
 			/* From BKDG, Write Leveling Seed Value. */
 			if (is_fam15h()) {
 				uint32_t RegisterDelay;
-				int32_t SeedTotal[lane_count];
+				int32_t SeedTotal[MAX_LANE_COUNT];
-				int32_t SeedTotalPreScaling[lane_count];
+				int32_t SeedTotalPreScaling[MAX_LANE_COUNT];
 				uint32_t WrDqDqsEarly;
 				uint8_t AddrCmdPrelaunch = 0;		/* TODO: Fetch the correct value from RC2[0] */
 

src/soc/intel/baytrail/spi.c

@@ -248,6 +248,8 @@ static ich9_spi_regs *spi_regs(void)
 	return (void *)sbase;
 }
 
+#define MENU_BYTES member_size(struct ich9_spi_regs, opmenu)
+
 void spi_init(void)
 {
 	ich9_spi_regs *ich9_spi = spi_regs();
@@ -332,7 +334,7 @@ static void spi_setup_type(spi_transaction *trans)
 static int spi_setup_opcode(spi_transaction *trans)
 {
 	uint16_t optypes;
-	uint8_t opmenu[cntlr.menubytes];
+	uint8_t opmenu[MENU_BYTES];
 
 	trans->opcode = trans->out[0];
 	spi_use_out(trans, 1);
@@ -353,13 +355,12 @@ static int spi_setup_opcode(spi_transaction *trans)
 			return 0;
 
 		read_reg(cntlr.opmenu, opmenu, sizeof(opmenu));
-		for (opcode_index = 0; opcode_index < cntlr.menubytes;
+		for (opcode_index = 0; opcode_index < ARRAY_SIZE(opmenu); opcode_index++) {
-				opcode_index++) {
 			if (opmenu[opcode_index] == trans->opcode)
 				break;
 		}
 
-		if (opcode_index == cntlr.menubytes) {
+		if (opcode_index == ARRAY_SIZE(opmenu)) {
 			printk(BIOS_DEBUG, "ICH SPI: Opcode %x not found\n",
 				trans->opcode);
 			return -1;

src/soc/intel/braswell/spi.c

@@ -221,6 +221,8 @@ static ich9_spi_regs *spi_regs(void)
 	return (void *)sbase;
 }
 
+#define MENU_BYTES member_size(struct ich9_spi_regs, opmenu)
+
 void spi_init(void)
 {
 	ich9_spi_regs *ich9_spi;
@@ -310,7 +312,7 @@ static void spi_setup_type(spi_transaction *trans)
 static int spi_setup_opcode(spi_transaction *trans)
 {
 	uint16_t optypes;
-	uint8_t opmenu[cntlr.menubytes];
+	uint8_t opmenu[MENU_BYTES];
 
 	trans->opcode = trans->out[0];
 	spi_use_out(trans, 1);
@@ -332,13 +334,12 @@ static int spi_setup_opcode(spi_transaction *trans)
 		return 0;
 
 	read_reg(cntlr.opmenu, opmenu, sizeof(opmenu));
-	for (opcode_index = 0; opcode_index < cntlr.menubytes;
+	for (opcode_index = 0; opcode_index < ARRAY_SIZE(opmenu); opcode_index++) {
-			opcode_index++) {
 		if (opmenu[opcode_index] == trans->opcode)
 			break;
 	}
 
-	if (opcode_index == cntlr.menubytes) {
+	if (opcode_index == ARRAY_SIZE(opmenu)) {
 		printk(BIOS_DEBUG, "ICH SPI: Opcode %x not found\n",
 			trans->opcode);
 		return -1;

src/soc/intel/broadwell/spi.c

@@ -231,6 +231,8 @@ static void ich_set_bbar(uint32_t minaddr)
 	writel_(ichspi_bbar, cntlr.bbar);
 }
 
+#define MENU_BYTES member_size(struct ich9_spi_regs, opmenu)
+
 void spi_init(void)
 {
 	uint8_t *rcrb; /* Root Complex Register Block */
@@ -332,7 +334,7 @@ static void spi_setup_type(spi_transaction *trans)
 static int spi_setup_opcode(spi_transaction *trans)
 {
 	uint16_t optypes;
-	uint8_t opmenu[cntlr.menubytes];
+	uint8_t opmenu[MENU_BYTES];
 
 	trans->opcode = trans->out[0];
 	spi_use_out(trans, 1);
@@ -354,13 +356,12 @@ static int spi_setup_opcode(spi_transaction *trans)
 		return 0;
 
 	read_reg(cntlr.opmenu, opmenu, sizeof(opmenu));
-	for (opcode_index = 0; opcode_index < cntlr.menubytes;
+	for (opcode_index = 0; opcode_index < ARRAY_SIZE(opmenu); opcode_index++) {
-			opcode_index++) {
 		if (opmenu[opcode_index] == trans->opcode)
 			break;
 	}
 
-	if (opcode_index == cntlr.menubytes) {
+	if (opcode_index == ARRAY_SIZE(opmenu)) {
 		printk(BIOS_DEBUG, "ICH SPI: Opcode %x not found\n",
 			trans->opcode);
 		return -1;

src/soc/intel/fsp_baytrail/spi.c

@@ -237,6 +237,8 @@ static ich9_spi_regs *spi_regs(void)
 	return (void *)sbase;
 }
 
+#define MENU_BYTES member_size(struct ich9_spi_regs, opmenu)
+
 void spi_init(void)
 {
 	ich_spi_controller *cntlr = car_get_var_ptr(&g_cntlr);
@@ -314,7 +316,7 @@ static int spi_setup_opcode(spi_transaction *trans)
 {
 	ich_spi_controller *cntlr = car_get_var_ptr(&g_cntlr);
 	uint16_t optypes;
-	uint8_t opmenu[cntlr->menubytes];
+	uint8_t opmenu[MENU_BYTES];
 
 	trans->opcode = trans->out[0];
 	spi_use_out(trans, 1);
@@ -335,13 +337,12 @@ static int spi_setup_opcode(spi_transaction *trans)
 			return 0;
 
 		read_reg(cntlr->opmenu, opmenu, sizeof(opmenu));
-		for (opcode_index = 0; opcode_index < cntlr->menubytes;
+		for (opcode_index = 0; opcode_index < ARRAY_SIZE(opmenu); opcode_index++) {
-				opcode_index++) {
 			if (opmenu[opcode_index] == trans->opcode)
 				break;
 		}
 
-		if (opcode_index == cntlr->menubytes) {
+		if (opcode_index == ARRAY_SIZE(opmenu)) {
 			printk(BIOS_DEBUG, "ICH SPI: Opcode %x not found\n",
 				trans->opcode);
 			return -1;

src/soc/intel/skylake/acpi.c

@@ -350,25 +350,26 @@ void acpi_fill_fadt(acpi_fadt_t *fadt)
 	fadt->x_gpe1_blk.addrh = 0x0;
 }
 
-static void generate_c_state_entries(int s0ix_enable, int max_cstate)
+static void write_c_state_entries(acpi_cstate_t *map, const int *set, size_t max_c_state)
 {
-
+	for (size_t i = 0; i < max_c_state; i++) {
-	acpi_cstate_t map[max_cstate];
-	int *set;
-	int i;
-
-	if (s0ix_enable)
-		set = cstate_set_s0ix;
-	else
-		set = cstate_set_non_s0ix;
-
-	for (i = 0; i < max_cstate; i++) {
 		memcpy(&map[i], &cstate_map[set[i]], sizeof(acpi_cstate_t));
 		map[i].ctype = i + 1;
 	}
 
 	/* Generate C-state tables */
-	acpigen_write_CST_package(map, ARRAY_SIZE(map));
+	acpigen_write_CST_package(map, max_c_state);
+}
+
+static void generate_c_state_entries(int s0ix_enable)
+{
+	if (s0ix_enable) {
+		acpi_cstate_t map[ARRAY_SIZE(cstate_set_s0ix)];
+		write_c_state_entries(map, cstate_set_s0ix, ARRAY_SIZE(map));
+	} else {
+		acpi_cstate_t map[ARRAY_SIZE(cstate_set_non_s0ix)];
+		write_c_state_entries(map, cstate_set_non_s0ix, ARRAY_SIZE(map));
+	}
 }
 
 static int calculate_power(int tdp, int p1_ratio, int ratio)
@@ -506,12 +507,6 @@ void generate_cpu_entries(struct device *device)
 	int numcpus = totalcores/cores_per_package;
 	config_t *config = config_of_path(SA_DEVFN_ROOT);
 	int is_s0ix_enable = config->s0ix_enable;
-	int max_c_state;
-
-	if (is_s0ix_enable)
-		max_c_state = ARRAY_SIZE(cstate_set_s0ix);
-	else
-		max_c_state = ARRAY_SIZE(cstate_set_non_s0ix);
 
 	printk(BIOS_DEBUG, "Found %d CPU(s) with %d core(s) each.\n",
 	       numcpus, cores_per_package);
@@ -534,8 +529,7 @@ void generate_cpu_entries(struct device *device)
 				cpu_id*cores_per_package+core_id,
 				pcontrol_blk, plen);
 			/* Generate C-state tables */
-			generate_c_state_entries(is_s0ix_enable,
+			generate_c_state_entries(is_s0ix_enable);
-				max_c_state);
 
 			if (config->eist_enable) {
 				/* Generate P-state tables */

src/southbridge/intel/common/spi.c

@@ -264,6 +264,12 @@ static void ich_set_bbar(uint32_t minaddr)
 	writel_(ichspi_bbar, cntlr->bbar);
 }
 
+#if CONFIG(SOUTHBRIDGE_INTEL_I82801GX)
+#define MENU_BYTES member_size(struct ich7_spi_regs, opmenu)
+#else
+#define MENU_BYTES member_size(struct ich9_spi_regs, opmenu)
+#endif
+
 void spi_init(void)
 {
 	struct ich_spi_controller *cntlr = &g_cntlr;
@@ -410,7 +416,7 @@ static int spi_setup_opcode(spi_transaction *trans)
 {
 	struct ich_spi_controller *cntlr = &g_cntlr;
 	uint16_t optypes;
-	uint8_t opmenu[cntlr->menubytes];
+	uint8_t opmenu[MENU_BYTES];
 
 	trans->opcode = trans->out[0];
 	spi_use_out(trans, 1);
@@ -432,13 +438,12 @@ static int spi_setup_opcode(spi_transaction *trans)
 		return 0;
 
 	read_reg(cntlr->opmenu, opmenu, sizeof(opmenu));
-	for (opcode_index = 0; opcode_index < cntlr->menubytes;
+	for (opcode_index = 0; opcode_index < ARRAY_SIZE(opmenu); opcode_index++) {
-			opcode_index++) {
 		if (opmenu[opcode_index] == trans->opcode)
 			break;
 	}
 
-	if (opcode_index == cntlr->menubytes) {
+	if (opcode_index == ARRAY_SIZE(opmenu)) {
 		printk(BIOS_DEBUG, "ICH SPI: Opcode %x not found\n",
 			trans->opcode);
 		return -1;

src/southbridge/intel/fsp_rangeley/spi.c

@@ -310,6 +310,8 @@ static inline int get_ich_version(uint16_t device_id)
 	return 0;
 }
 
+#define MENU_BYTES member_size(struct ich10_spi_regs, opmenu)
+
 void spi_init(void)
 {
 	int ich_version = 0;
@@ -444,7 +446,7 @@ static void spi_setup_type(spi_transaction *trans)
 static int spi_setup_opcode(spi_transaction *trans)
 {
 	uint16_t optypes;
-	uint8_t opmenu[cntlr.menubytes];
+	uint8_t opmenu[MENU_BYTES];
 
 	trans->opcode = trans->out[0];
 	spi_use_out(trans, 1);
@@ -465,13 +467,12 @@ static int spi_setup_opcode(spi_transaction *trans)
 			return 0;
 
 		read_reg(cntlr.opmenu, opmenu, sizeof(opmenu));
-		for (opcode_index = 0; opcode_index < cntlr.menubytes;
+		for (opcode_index = 0; opcode_index < ARRAY_SIZE(opmenu); opcode_index++) {
-				opcode_index++) {
 			if (opmenu[opcode_index] == trans->opcode)
 				break;
 		}
 
-		if (opcode_index == cntlr.menubytes) {
+		if (opcode_index == ARRAY_SIZE(opmenu)) {
 			printk(BIOS_DEBUG, "ICH SPI: Opcode %x not found\n",
 				trans->opcode);
 			return -1;

src/vendorcode/google/chromeos/sar.c

@@ -61,11 +61,10 @@ int get_wifi_sar_limits(struct wifi_sar_limits *sar_limits)
 	const char *wifi_sar_limit_key = CROS_VPD_WIFI_SAR_NAME;
 	/* vpd_gets() reads in one less than size characters from the VPD
 	 * with a terminating null byte ('\0') stored as the last character into
-	 * the buffer, thus the increasing by 1 for buffer_size. */
+	 * the buffer, thus the increasing by 1 for the buffer size. */
-	const size_t buffer_size = (sizeof(struct wifi_sar_limits) /
+	char wifi_sar_limit_str[2 * sizeof(struct wifi_sar_limits) + 1];
-					sizeof(uint8_t)) * 2 + 1;
-	char wifi_sar_limit_str[buffer_size];
 	uint8_t bin_buffer[sizeof(struct wifi_sar_limits)];
+	const size_t buffer_size = ARRAY_SIZE(wifi_sar_limit_str);
 	size_t sar_cbfs_len, sar_expected_len, bin_buff_adjusted_size;
 
 	/* keep it backward compatible. Some older platform are shipping