diff options
author | Jacob Garber <jgarber1@ualberta.ca> | 2019-06-26 16:18:16 -0600 |
---|---|---|
committer | Patrick Georgi <pgeorgi@google.com> | 2019-08-20 15:27:42 +0000 |
commit | 9172b6920cac2c4dabf19e529dbfed91b15685c5 (patch) | |
tree | 761b138ce45fface88f8babba31d48bff43203d5 /src/southbridge | |
parent | 5fa756cc97de1ed30ac3fd4d5ddb85f079efe521 (diff) |
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>
Diffstat (limited to 'src/southbridge')
-rw-r--r-- | src/southbridge/intel/common/spi.c | 13 | ||||
-rw-r--r-- | src/southbridge/intel/fsp_rangeley/spi.c | 9 |
2 files changed, 14 insertions, 8 deletions
diff --git a/src/southbridge/intel/common/spi.c b/src/southbridge/intel/common/spi.c index 0095dd9ccb..65693512f5 100644 --- a/src/southbridge/intel/common/spi.c +++ b/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; - 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 afd89a7dba..d2f2a0bd32 100644 --- a/src/southbridge/intel/fsp_rangeley/spi.c +++ b/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; - 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; |