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 <th3fanbus@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/50480
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Julius Werner <jwerner@chromium.org>
Reviewed-by: Jacob Garber <jgarber1@ualberta.ca>
Reviewed-by: Paul Menzel <paulepanter@users.sourceforge.net>
This commit is contained in:
Angel Pons 2021-02-10 16:07:44 +01:00 committed by Patrick Georgi
parent 9cf9b85476
commit 59626b8670
2 changed files with 8 additions and 8 deletions

View File

@ -88,17 +88,15 @@ int spi_flash_cmd(const struct spi_slave *spi, u8 cmd, void *response, size_t le
return ret; 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, int spi_flash_cmd_write(const struct spi_slave *spi, const u8 *cmd,
size_t cmd_len, const void *data, size_t data_len) size_t cmd_len, const void *data, size_t data_len)
{ {
int ret; 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, cmd_len);
memcpy(buff + cmd_len, data, data_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; return ret;
} }
#pragma GCC diagnostic pop
/* Perform the read operation honoring spi controller fifo size, reissuing /* Perform the read operation honoring spi controller fifo size, reissuing
* the read command until the full request completed. */ * 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; byte_addr = offset % page_size;
chunk_len = MIN(len - actual, page_size - byte_addr); chunk_len = MIN(len - actual, page_size - byte_addr);
chunk_len = spi_crop_chunk(&flash->spi, sizeof(cmd), chunk_len); 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); spi_flash_addr(offset, cmd);
if (CONFIG(DEBUG_SPI_FLASH)) { if (CONFIG(DEBUG_SPI_FLASH)) {

View File

@ -24,6 +24,8 @@
/* Common status */ /* Common status */
#define STATUS_WIP 0x01 #define STATUS_WIP 0x01
#define MAX_FLASH_CMD_DATA_SIZE 256
/* Send a single-byte command to the device and read the response */ /* 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); int spi_flash_cmd(const struct spi_slave *spi, u8 cmd, void *response, size_t len);