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

[Patrick Georgi (Code Review)]

Patrick Georgi has submitted this change. ( 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. Thus, the maximum page size is currently 256.

Define the `MAX_FLASH_CMD_DATA_SIZE` macro to place an upper bound on
the amount of data that can be written in one command. Then, use this
value to allocate a fixed-size buffer in `spi_flash_cmd_write`. Also,
add a check to prevent buffer overflow problems. Finally, ensure that
the `spi_flash_cmd_write_page_program` function always writes no more
than 256 bytes of data when using the `spi_flash_cmd_write` function.

Tested on Asrock B85M Pro4 (Winbond W25Q64FV), MRC cache still works.

Change-Id: Ib630bff1b496bc276616989d4506a3c96f242e26
Signed-off-by: Angel Pons 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/50480
Tested-by: build bot (Jenkins) 
Reviewed-by: Julius Werner 
Reviewed-by: Jacob Garber 
Reviewed-by: Paul Menzel 
---
M src/drivers/spi/spi_flash.c
M src/drivers/spi/spi_flash_internal.h
2 files changed, 8 insertions(+), 8 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Paul Menzel: Looks good to me, but someone else must approve
  Julius Werner: Looks good to me, approved
  Jacob Garber: Looks good to me, but someone else must approve

diff --git a/src/drivers/spi/spi_flash.c b/src/drivers/spi/spi_flash.c
index b674749..47b4b85 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 + MAX_FLASH_CMD_DATA_SIZE];
+
+	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. */
@@ -259,6 +256,7 @@
    	byte_addr = offset % page_size;
    	chunk_len = MIN(len - actual, page_size - byte_addr);
    	chunk_len = spi_crop_chunk(&flash->spi, sizeof(cmd), chunk_len);
+		chunk_len = MIN(MAX_FLASH_CMD_DATA_SIZE, chunk_len);

spi_flash_addr(offset, cmd);
    	if (CONFIG(DEBUG_SPI_FLASH)) {
diff --git a/src/drivers/spi/spi_flash_internal.h b/src/drivers/spi/spi_flash_internal.h
index b4d39b3..7fe3ea6 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 MAX_FLASH_CMD_DATA_SIZE		256
+
 /* 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);