Change in coreboot[master]: drivers/spi: Stop using a variable-length array - coreboot-gerrit

10 Feb 2021

Angel Pons has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/50480 )
Change subject: drivers/spi: Stop using a variable-length array
......................................................................
drivers/spi: Stop using a variable-length array
Only the call in `spi_flash_cmd_write_page_program` uses non-constant
values for the array length. However, the value for `data_len` has an
upper bound: `flash->page_size` is set to `1U << vi->page_size_shift`
which depends on the flash chip vendor info, and the largest value it
can currently have is 8.
Define this number in a macro, and use it to allocate a fixed buffer.
Also add an overflow check to guard against buffer overflow problems.
Tested on Asrock B85M Pro4, MRC cache is still working.
Change-Id: Ib630bff1b496bc276616989d4506a3c96f242e26
Signed-off-by: Angel Pons th3fanbus@gmail.com
---
M src/drivers/spi/adesto.c
M src/drivers/spi/amic.c
M src/drivers/spi/atmel.c
M src/drivers/spi/eon.c
M src/drivers/spi/gigadevice.c
M src/drivers/spi/macronix.c
M src/drivers/spi/spansion.c
M src/drivers/spi/spi_flash.c
M src/drivers/spi/spi_flash_internal.h
M src/drivers/spi/sst.c
M src/drivers/spi/stmicro.c
M src/drivers/spi/winbond.c
12 files changed, 22 insertions(+), 23 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/80/50480/1

diff --git a/src/drivers/spi/adesto.c b/src/drivers/spi/adesto.c
index 6538905..aa31d92 100644
--- a/src/drivers/spi/adesto.c
+++ b/src/drivers/spi/adesto.c
@@ -91,7 +91,7 @@
const struct spi_flash_vendor_info spi_flash_adesto_vi = {
    .id = VENDOR_ID_ADESTO,
-	.page_size_shift = 8,
+	.page_size_shift = DEFAULT_PAGE_SIZE_SHIFT,
    .sector_size_kib_shift = 2,
    .match_id_mask[0] = 0xffff,
    .ids = flash_table,
diff --git a/src/drivers/spi/amic.c b/src/drivers/spi/amic.c
index 5e74616..40d11ba 100644
--- a/src/drivers/spi/amic.c
+++ b/src/drivers/spi/amic.c
@@ -71,7 +71,7 @@
const struct spi_flash_vendor_info spi_flash_amic_vi = {
    .id = VENDOR_ID_AMIC,
-	.page_size_shift = 8,
+	.page_size_shift = DEFAULT_PAGE_SIZE_SHIFT,
    .sector_size_kib_shift = 2,
    .match_id_mask[0] = 0xffff,
    .ids = flash_table,
diff --git a/src/drivers/spi/atmel.c b/src/drivers/spi/atmel.c
index 4dcc5b5..c8a7b0e 100644
--- a/src/drivers/spi/atmel.c
+++ b/src/drivers/spi/atmel.c
@@ -61,7 +61,7 @@
const struct spi_flash_vendor_info spi_flash_atmel_vi = {
    .id = VENDOR_ID_ATMEL,
-	.page_size_shift = 8,
+	.page_size_shift = DEFAULT_PAGE_SIZE_SHIFT,
    .sector_size_kib_shift = 2,
    .match_id_mask[0] = 0xffff,
    .ids = flash_table,
diff --git a/src/drivers/spi/eon.c b/src/drivers/spi/eon.c
index 1212c6b..89b6318 100644
--- a/src/drivers/spi/eon.c
+++ b/src/drivers/spi/eon.c
@@ -152,7 +152,7 @@
const struct spi_flash_vendor_info spi_flash_eon_vi = {
    .id = VENDOR_ID_EON,
-	.page_size_shift = 8,
+	.page_size_shift = DEFAULT_PAGE_SIZE_SHIFT,
    .sector_size_kib_shift = 2,
    .match_id_mask[0] = 0xffff,
    .ids = flash_table,
diff --git a/src/drivers/spi/gigadevice.c b/src/drivers/spi/gigadevice.c
index 6c5a167..dabfd6a 100644
--- a/src/drivers/spi/gigadevice.c
+++ b/src/drivers/spi/gigadevice.c
@@ -103,7 +103,7 @@
const struct spi_flash_vendor_info spi_flash_gigadevice_vi = {
    .id = VENDOR_ID_GIGADEVICE,
-	.page_size_shift = 8,
+	.page_size_shift = DEFAULT_PAGE_SIZE_SHIFT,
    .sector_size_kib_shift = 2,
    .match_id_mask[0] = 0xffff,
    .ids = flash_table,
diff --git a/src/drivers/spi/macronix.c b/src/drivers/spi/macronix.c
index f3f7e2d..4485752 100644
--- a/src/drivers/spi/macronix.c
+++ b/src/drivers/spi/macronix.c
@@ -123,7 +123,7 @@
const struct spi_flash_vendor_info spi_flash_macronix_vi = {
    .id = VENDOR_ID_MACRONIX,
-	.page_size_shift = 8,
+	.page_size_shift = DEFAULT_PAGE_SIZE_SHIFT,
    .sector_size_kib_shift = 2,
    .match_id_mask[0] = 0xffff,
    .ids = flash_table,
diff --git a/src/drivers/spi/spansion.c b/src/drivers/spi/spansion.c
index fe0e265..ee4b8ed 100644
--- a/src/drivers/spi/spansion.c
+++ b/src/drivers/spi/spansion.c
@@ -117,7 +117,7 @@
const struct spi_flash_vendor_info spi_flash_spansion_ext1_vi = {
    .id = VENDOR_ID_SPANSION,
-	.page_size_shift = 8,
+	.page_size_shift = DEFAULT_PAGE_SIZE_SHIFT,
    .sector_size_kib_shift = 6,
    .match_id_mask[0] = 0xffff,
    .match_id_mask[1] = 0xffff,
@@ -128,7 +128,7 @@
const struct spi_flash_vendor_info spi_flash_spansion_ext2_vi = {
    .id = VENDOR_ID_SPANSION,
-	.page_size_shift = 8,
+	.page_size_shift = DEFAULT_PAGE_SIZE_SHIFT,
    .sector_size_kib_shift = 8,
    .match_id_mask[0] = 0xffff,
    .match_id_mask[1] = 0xffff,
@@ -139,7 +139,7 @@
const struct spi_flash_vendor_info spi_flash_spansion_vi = {
    .id = VENDOR_ID_SPANSION,
-	.page_size_shift = 8,
+	.page_size_shift = DEFAULT_PAGE_SIZE_SHIFT,
    .sector_size_kib_shift = 6,
    .match_id_mask[0] = 0xffff,
    .ids = flash_table,
diff --git a/src/drivers/spi/spi_flash.c b/src/drivers/spi/spi_flash.c
index b674749..aa0da70 100644
--- a/src/drivers/spi/spi_flash.c
+++ b/src/drivers/spi/spi_flash.c
@@ -88,17 +88,15 @@
    return ret;
 }
-/* TODO: This code is quite possibly broken and overflowing stacks. Fix ASAP! */
-#pragma GCC diagnostic push
-#if defined(__GNUC__) && !defined(__clang__)
-#pragma GCC diagnostic ignored "-Wstack-usage="
-#endif
-#pragma GCC diagnostic ignored "-Wvla"
 int spi_flash_cmd_write(const struct spi_slave *spi, const u8 *cmd,
    		size_t cmd_len, const void *data, size_t data_len)
 {
    int ret;
-	u8 buff[cmd_len + data_len];
+	u8 buff[4 + (1 << DEFAULT_PAGE_SIZE_SHIFT)];
+
+	if (ARRAY_SIZE(buff) < cmd_len + data_len)
+		return -1;
+
    memcpy(buff, cmd, cmd_len);
    memcpy(buff + cmd_len, data, data_len);
@@ -110,7 +108,6 @@
return ret;
 }
-#pragma GCC diagnostic pop
/* Perform the read operation honoring spi controller fifo size, reissuing
  * the read command until the full request completed. */
diff --git a/src/drivers/spi/spi_flash_internal.h b/src/drivers/spi/spi_flash_internal.h
index b4d39b3..e656498 100644
--- a/src/drivers/spi/spi_flash_internal.h
+++ b/src/drivers/spi/spi_flash_internal.h
@@ -24,6 +24,8 @@
 /* Common status */
 #define STATUS_WIP			0x01
+#define DEFAULT_PAGE_SIZE_SHIFT		8
+
 /* Send a single-byte command to the device and read the response */
 int spi_flash_cmd(const struct spi_slave *spi, u8 cmd, void *response, size_t len);
diff --git a/src/drivers/spi/sst.c b/src/drivers/spi/sst.c
index 887380f..8c20a3e 100644
--- a/src/drivers/spi/sst.c
+++ b/src/drivers/spi/sst.c
@@ -250,7 +250,7 @@
const struct spi_flash_vendor_info spi_flash_sst_vi = {
    .id = VENDOR_ID_SST,
-	.page_size_shift = 8,
+	.page_size_shift = DEFAULT_PAGE_SIZE_SHIFT,
    .sector_size_kib_shift = 2,
    .match_id_mask[0] = 0xff,
    .ids = flash_table_pp256,
diff --git a/src/drivers/spi/stmicro.c b/src/drivers/spi/stmicro.c
index 4cd8c1b..4fb0569 100644
--- a/src/drivers/spi/stmicro.c
+++ b/src/drivers/spi/stmicro.c
@@ -213,7 +213,7 @@
const struct spi_flash_vendor_info spi_flash_stmicro1_vi = {
    .id = VENDOR_ID_STMICRO,
-	.page_size_shift = 8,
+	.page_size_shift = DEFAULT_PAGE_SIZE_SHIFT,
    .sector_size_kib_shift = 5,
    .match_id_mask[0] = 0xffff,
    .ids = flash_table_se32k,
@@ -223,7 +223,7 @@
const struct spi_flash_vendor_info spi_flash_stmicro2_vi = {
    .id = VENDOR_ID_STMICRO,
-	.page_size_shift = 8,
+	.page_size_shift = DEFAULT_PAGE_SIZE_SHIFT,
    .sector_size_kib_shift = 6,
    .match_id_mask[0] = 0xffff,
    .ids = flash_table_se64k,
@@ -233,7 +233,7 @@
const struct spi_flash_vendor_info spi_flash_stmicro3_vi = {
    .id = VENDOR_ID_STMICRO,
-	.page_size_shift = 8,
+	.page_size_shift = DEFAULT_PAGE_SIZE_SHIFT,
    .sector_size_kib_shift = 8,
    .match_id_mask[0] = 0xffff,
    .ids = flash_table_se256k,
@@ -243,7 +243,7 @@
const struct spi_flash_vendor_info spi_flash_stmicro4_vi = {
    .id = VENDOR_ID_STMICRO,
-	.page_size_shift = 8,
+	.page_size_shift = DEFAULT_PAGE_SIZE_SHIFT,
    .sector_size_kib_shift = 2,
    .match_id_mask[0] = 0xffff,
    .ids = flash_table_sse,
diff --git a/src/drivers/spi/winbond.c b/src/drivers/spi/winbond.c
index e4151de..e3cc33d 100644
--- a/src/drivers/spi/winbond.c
+++ b/src/drivers/spi/winbond.c
@@ -533,7 +533,7 @@
const struct spi_flash_vendor_info spi_flash_winbond_vi = {
    .id = VENDOR_ID_WINBOND,
-	.page_size_shift = 8,
+	.page_size_shift = DEFAULT_PAGE_SIZE_SHIFT,
    .sector_size_kib_shift = 2,
    .match_id_mask[0] = 0xffff,
    .ids = flash_table,
-- 
To view, visit https://review.coreboot.org/c/coreboot/+/50480
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ib630bff1b496bc276616989d4506a3c96f242e26
Gerrit-Change-Number: 50480
Gerrit-PatchSet: 1
Gerrit-Owner: Angel Pons th3fanbus@gmail.com
Gerrit-MessageType: newchange



    