From e19b5e7acde3990de9f7dc25e1b43f07dc0cb0fa Mon Sep 17 00:00:00 2001 From: Felix Held Date: Tue, 6 Aug 2024 01:01:08 +0200 Subject: soc/amd/common/psp_smi_flash: add buffer overflow checks Before 'handle_psp_command' calls any of the functions in this file, it make sure that the 'size' field in the command buffer's header doesn't indicate that the command buffer is larger than the SMM memory region reserved for it. The read/write command buffer has a 'num_bytes' field to indicate how many bytes should be read from the SPI flash and put into the data buffer within the command buffer or how many bytes from this buffer should be written to the flash. While we should be able to assume that the PSP won't send us malformed command buffer, we should still better check this just to be sure. Test=When selecting SOC_AMD_COMMON_BLOCK_PSP_SMI, Mandolin still builds Signed-off-by: Felix Held Change-Id: Ib4e8514eedc3ad154a705c8a1e85d367e452dbed Reviewed-on: https://review.coreboot.org/c/coreboot/+/83778 Reviewed-by: Martin Roth Tested-by: build bot (Jenkins) --- src/soc/amd/common/block/psp/psp_smi_flash.c | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) (limited to 'src/soc/amd/common/block') diff --git a/src/soc/amd/common/block/psp/psp_smi_flash.c b/src/soc/amd/common/block/psp/psp_smi_flash.c index 2d075eff84..405fc29ef7 100644 --- a/src/soc/amd/common/block/psp/psp_smi_flash.c +++ b/src/soc/amd/common/block/psp/psp_smi_flash.c @@ -104,6 +104,16 @@ static void get_psp_spi_erase(struct mbox_pspv2_cmd_spi_erase *cmd_buf, *num_blocks = read64(&cmd_buf->req.num_blocks); } +static bool is_valid_rw_byte_count(struct mbox_pspv2_cmd_spi_read_write *cmd_buf, + u64 num_bytes) +{ + const u32 cmd_buf_size = read32(&cmd_buf->header.size); + const size_t payload_buffer_offset = + offsetof(struct mbox_pspv2_cmd_spi_read_write, req) + + offsetof(struct pspv2_spi_read_write_request, buffer); + return num_bytes <= cmd_buf_size - payload_buffer_offset; +} + static const char *id_to_region_name(u64 target_nv_id) { switch (target_nv_id) { @@ -238,6 +248,12 @@ enum mbox_p2c_status psp_smi_spi_read(struct mbox_default_buffer *buffer) get_psp_spi_read_write(cmd_buf, &target_nv_id, &lba, &offset, &num_bytes, &data); + if (!is_valid_rw_byte_count(cmd_buf, num_bytes)) { + printk(BIOS_ERR, "PSP: Read command requested more bytes than we have space " + "for in the buffer\n"); + return MBOX_PSP_COMMAND_PROCESS_ERROR; + } + ret = find_psp_spi_flash_device_region(target_nv_id, &store, &flash); if (ret != MBOX_PSP_SUCCESS) @@ -281,6 +297,12 @@ enum mbox_p2c_status psp_smi_spi_write(struct mbox_default_buffer *buffer) get_psp_spi_read_write(cmd_buf, &target_nv_id, &lba, &offset, &num_bytes, &data); + if (!is_valid_rw_byte_count(cmd_buf, num_bytes)) { + printk(BIOS_ERR, "PSP: Write command contains more bytes than we have space " + "for in the buffer\n"); + return MBOX_PSP_COMMAND_PROCESS_ERROR; + } + ret = find_psp_spi_flash_device_region(target_nv_id, &store, &flash); if (ret != MBOX_PSP_SUCCESS) -- cgit v1.2.3