From 24cb2e0d57cea0cbc163f23fa47d530b35425a21 Mon Sep 17 00:00:00 2001 From: Jean-Christophe Dubois Date: Mon, 9 Jan 2017 11:40:23 +0000 Subject: [PATCH] m25p80: don't let rogue SPI controllers cause buffer overruns In normal operation we should never attempt to put more data into the data[] array than it can hold. However if the SPI controller connected to us misbehaves then it can send us a sequence of commands that attempt this. Since the controller might be in the guest (if the hardware does SPI via bit-banging), catch the possible overrun conditions and reset the flash internal state, logging them as guest errors. Signed-off-by: Jean-Christophe Dubois Message-id: 20170107111631.24444-1-jcd@tribudubois.net Reviewed-by: Peter Maydell [PMM: rewrote commit message to be more exact about when this can happen] Signed-off-by: Peter Maydell --- hw/block/m25p80.c | 29 +++++++++++++++++++++++++++-- 1 file changed, 27 insertions(+), 2 deletions(-) diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c index e3c1166ea6..4c5f8c3590 100644 --- a/hw/block/m25p80.c +++ b/hw/block/m25p80.c @@ -28,6 +28,7 @@ #include "hw/ssi/ssi.h" #include "qemu/bitops.h" #include "qemu/log.h" +#include "qemu/error-report.h" #include "qapi/error.h" #ifndef M25P80_ERR_DEBUG @@ -377,6 +378,8 @@ typedef enum { MAN_GENERIC, } Manufacturer; +#define M25P80_INTERNAL_DATA_BUFFER_SZ 16 + typedef struct Flash { SSISlave parent_obj; @@ -387,7 +390,7 @@ typedef struct Flash { int page_size; uint8_t state; - uint8_t data[16]; + uint8_t data[M25P80_INTERNAL_DATA_BUFFER_SZ]; uint32_t len; uint32_t pos; uint8_t needed_bytes; @@ -1115,6 +1118,17 @@ static uint32_t m25p80_transfer8(SSISlave *ss, uint32_t tx) case STATE_COLLECTING_DATA: case STATE_COLLECTING_VAR_LEN_DATA: + + if (s->len >= M25P80_INTERNAL_DATA_BUFFER_SZ) { + qemu_log_mask(LOG_GUEST_ERROR, + "M25P80: Write overrun internal data buffer. " + "SPI controller (QEMU emulator or guest driver) " + "is misbehaving\n"); + s->len = s->pos = 0; + s->state = STATE_IDLE; + break; + } + s->data[s->len] = (uint8_t)tx; s->len++; @@ -1124,6 +1138,17 @@ static uint32_t m25p80_transfer8(SSISlave *ss, uint32_t tx) break; case STATE_READING_DATA: + + if (s->pos >= M25P80_INTERNAL_DATA_BUFFER_SZ) { + qemu_log_mask(LOG_GUEST_ERROR, + "M25P80: Read overrun internal data buffer. " + "SPI controller (QEMU emulator or guest driver) " + "is misbehaving\n"); + s->len = s->pos = 0; + s->state = STATE_IDLE; + break; + } + r = s->data[s->pos]; s->pos++; if (s->pos == s->len) { @@ -1196,7 +1221,7 @@ static const VMStateDescription vmstate_m25p80 = { .pre_save = m25p80_pre_save, .fields = (VMStateField[]) { VMSTATE_UINT8(state, Flash), - VMSTATE_UINT8_ARRAY(data, Flash, 16), + VMSTATE_UINT8_ARRAY(data, Flash, M25P80_INTERNAL_DATA_BUFFER_SZ), VMSTATE_UINT32(len, Flash), VMSTATE_UINT32(pos, Flash), VMSTATE_UINT8(needed_bytes, Flash), -- 2.11.4.GIT