From 59626b8670da326ab725e67b01dd1025b0a34a86 Mon Sep 17 00:00:00 2001 From: Angel Pons Date: Wed, 10 Feb 2021 16:07:44 +0100 Subject: 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 --- src/drivers/spi/spi_flash.c | 14 ++++++-------- src/drivers/spi/spi_flash_internal.h | 2 ++ 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/drivers/spi/spi_flash.c b/src/drivers/spi/spi_flash.c index b674749feb..47b4b85b0d 100644 --- a/src/drivers/spi/spi_flash.c +++ b/src/drivers/spi/spi_flash.c @@ -88,17 +88,15 @@ int spi_flash_cmd(const struct spi_slave *spi, u8 cmd, void *response, size_t le 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 @@ int spi_flash_cmd_write(const struct spi_slave *spi, const u8 *cmd, 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 @@ int spi_flash_cmd_write_page_program(const struct spi_flash *flash, u32 offset, 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 b4d39b3d31..7fe3ea6038 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); -- cgit v1.2.3