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>
diff --git a/src/arch/x86/smbios.c b/src/arch/x86/smbios.c
index 346e874..f95516e 100644
--- a/src/arch/x86/smbios.c
+++ b/src/arch/x86/smbios.c
@@ -275,20 +275,18 @@
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);
- trimmed_part_number[trimmed_buffer_size - 1] = '\0';
+ strncpy(trimmed_part_number, part_number, sizeof(trimmed_part_number));
+ 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 @@
/* 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 +
- 10 /* strlen("Invalid ()") */];
+ char string_buffer[sizeof(trimmed_part_number) + 10];
snprintf(string_buffer, sizeof(string_buffer), "Invalid (%s)",
trimmed_part_number);
diff --git a/src/drivers/spi/spi_flash.c b/src/drivers/spi/spi_flash.c
index cfa500e..e2485ab 100644
--- a/src/drivers/spi/spi_flash.c
+++ b/src/drivers/spi/spi_flash.c
@@ -100,6 +100,7 @@
#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)
diff --git a/src/northbridge/amd/amdmct/mct_ddr3/mctdqs_d.c b/src/northbridge/amd/amdmct/mct_ddr3/mctdqs_d.c
index 3f4f1bf..d34b2dc 100644
--- a/src/northbridge/amd/amdmct/mct_ddr3/mctdqs_d.c
+++ b/src/northbridge/amd/amdmct/mct_ddr3/mctdqs_d.c
@@ -1252,11 +1252,15 @@
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 @@
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 @@
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 @@
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 @@
/* 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]];
diff --git a/src/northbridge/amd/amdmct/mct_ddr3/mhwlc_d.c b/src/northbridge/amd/amdmct/mct_ddr3/mhwlc_d.c
index 76f72ae..353aa7a 100644
--- a/src/northbridge/amd/amdmct/mct_ddr3/mhwlc_d.c
+++ b/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 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 @@
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 @@
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 @@
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 @@
/* From BKDG, Write Leveling Seed Value. */
if (is_fam15h()) {
uint32_t RegisterDelay;
- int32_t SeedTotal[lane_count];
- int32_t SeedTotalPreScaling[lane_count];
+ int32_t SeedTotal[MAX_LANE_COUNT];
+ int32_t SeedTotalPreScaling[MAX_LANE_COUNT];
uint32_t WrDqDqsEarly;
uint8_t AddrCmdPrelaunch = 0; /* TODO: Fetch the correct value from RC2[0] */
diff --git a/src/soc/intel/baytrail/spi.c b/src/soc/intel/baytrail/spi.c
index d5b962f..26b717c 100644
--- a/src/soc/intel/baytrail/spi.c
+++ b/src/soc/intel/baytrail/spi.c
@@ -248,6 +248,8 @@
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 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 @@
return 0;
read_reg(cntlr.opmenu, opmenu, sizeof(opmenu));
- for (opcode_index = 0; opcode_index < cntlr.menubytes;
- opcode_index++) {
+ for (opcode_index = 0; opcode_index < ARRAY_SIZE(opmenu); 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;
diff --git a/src/soc/intel/braswell/spi.c b/src/soc/intel/braswell/spi.c
index b968283..00ec48f 100644
--- a/src/soc/intel/braswell/spi.c
+++ b/src/soc/intel/braswell/spi.c
@@ -221,6 +221,8 @@
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 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 @@
return 0;
read_reg(cntlr.opmenu, opmenu, sizeof(opmenu));
- for (opcode_index = 0; opcode_index < cntlr.menubytes;
- opcode_index++) {
+ for (opcode_index = 0; opcode_index < ARRAY_SIZE(opmenu); 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;
diff --git a/src/soc/intel/broadwell/spi.c b/src/soc/intel/broadwell/spi.c
index 01d2830..ac893ea 100644
--- a/src/soc/intel/broadwell/spi.c
+++ b/src/soc/intel/broadwell/spi.c
@@ -231,6 +231,8 @@
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 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 @@
return 0;
read_reg(cntlr.opmenu, opmenu, sizeof(opmenu));
- for (opcode_index = 0; opcode_index < cntlr.menubytes;
- opcode_index++) {
+ for (opcode_index = 0; opcode_index < ARRAY_SIZE(opmenu); 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;
diff --git a/src/soc/intel/fsp_baytrail/spi.c b/src/soc/intel/fsp_baytrail/spi.c
index 9375d19..0b52ea96 100644
--- a/src/soc/intel/fsp_baytrail/spi.c
+++ b/src/soc/intel/fsp_baytrail/spi.c
@@ -237,6 +237,8 @@
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 @@
{
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 @@
return 0;
read_reg(cntlr->opmenu, opmenu, sizeof(opmenu));
- for (opcode_index = 0; opcode_index < cntlr->menubytes;
- opcode_index++) {
+ for (opcode_index = 0; opcode_index < ARRAY_SIZE(opmenu); 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;
diff --git a/src/soc/intel/skylake/acpi.c b/src/soc/intel/skylake/acpi.c
index 869ca7f..aa51cbe 100644
--- a/src/soc/intel/skylake/acpi.c
+++ b/src/soc/intel/skylake/acpi.c
@@ -350,25 +350,26 @@
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)
{
-
- 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++) {
+ for (size_t i = 0; i < max_c_state; 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 @@
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 @@
cpu_id*cores_per_package+core_id,
pcontrol_blk, plen);
/* Generate C-state tables */
- generate_c_state_entries(is_s0ix_enable,
- max_c_state);
+ generate_c_state_entries(is_s0ix_enable);
if (config->eist_enable) {
/* Generate P-state tables */
diff --git a/src/southbridge/intel/common/spi.c b/src/southbridge/intel/common/spi.c
index 0095dd9..6569351 100644
--- a/src/southbridge/intel/common/spi.c
+++ b/src/southbridge/intel/common/spi.c
@@ -264,6 +264,12 @@
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 @@
{
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 @@
return 0;
read_reg(cntlr->opmenu, opmenu, sizeof(opmenu));
- for (opcode_index = 0; opcode_index < cntlr->menubytes;
- opcode_index++) {
+ for (opcode_index = 0; opcode_index < ARRAY_SIZE(opmenu); 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;
diff --git a/src/southbridge/intel/fsp_rangeley/spi.c b/src/southbridge/intel/fsp_rangeley/spi.c
index afd89a7..d2f2a0b 100644
--- a/src/southbridge/intel/fsp_rangeley/spi.c
+++ b/src/southbridge/intel/fsp_rangeley/spi.c
@@ -310,6 +310,8 @@
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 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 @@
return 0;
read_reg(cntlr.opmenu, opmenu, sizeof(opmenu));
- for (opcode_index = 0; opcode_index < cntlr.menubytes;
- opcode_index++) {
+ for (opcode_index = 0; opcode_index < ARRAY_SIZE(opmenu); 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;
diff --git a/src/vendorcode/google/chromeos/sar.c b/src/vendorcode/google/chromeos/sar.c
index bbcb211..01de60c 100644
--- a/src/vendorcode/google/chromeos/sar.c
+++ b/src/vendorcode/google/chromeos/sar.c
@@ -61,11 +61,10 @@
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. */
- const size_t buffer_size = (sizeof(struct wifi_sar_limits) /
- sizeof(uint8_t)) * 2 + 1;
- char wifi_sar_limit_str[buffer_size];
+ * the buffer, thus the increasing by 1 for the buffer size. */
+ char wifi_sar_limit_str[2 * sizeof(struct wifi_sar_limits) + 1];
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