[Qemu-devel] [PATCH v2] [m25p80] Reset internal state in case we overrun the internal data buffer

[Qemu-devel] [PATCH v2] [m25p80] Reset internal state in case we overrun the internal data buffer

[Jean-Christophe Dubois]

When we overrun the internal data buffer it is suspected that the SPI
controler (either Qemu emulator og guest driver) is misbehaving.

Therefore we reset the flash internal state and issue a log.

Signed-off-by: Jean-Christophe Dubois <jcd@tribudubois.net>
---

Changes since v1:
* reset internal state and issue a log instead of aborting.

 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 d29ff4c..6bbedd9 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
@@ -376,6 +377,8 @@ typedef enum {
     MAN_GENERIC,
 } Manufacturer;
 
+#define M25P80_INTERNAL_DATA_BUFFER_SZ 16
+
 typedef struct Flash {
     SSISlave parent_obj;
 
@@ -386,7 +389,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;
@@ -1114,6 +1117,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++;
 
@@ -1123,6 +1137,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) {
@@ -1195,7 +1220,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.9.3

Re: [Qemu-devel] [PATCH v2] [m25p80] Reset internal state in case we overrun the internal data buffer

[Peter Maydell]

On 7 January 2017 at 11:16, Jean-Christophe Dubois <jcd@tribudubois.net> wrote:
> When we overrun the internal data buffer it is suspected that the SPI
> controler (either Qemu emulator og guest driver) is misbehaving.
>
> Therefore we reset the flash internal state and issue a log.
>
> Signed-off-by: Jean-Christophe Dubois <jcd@tribudubois.net>
> ---
>
> Changes since v1:
> * reset internal state and issue a log instead of aborting.
>
>  hw/block/m25p80.c | 29 +++++++++++++++++++++++++++--
>  1 file changed, 27 insertions(+), 2 deletions(-)
>



Applied to target-arm.next, thanks.

-- PMM