[1/1] ARM: PL061: Checking register r/w accesses to reserved area

Commit Message

Wei Huang Feb. 18, 2016, 4:56 p.m. UTC

pl061.c emulates two GPIO devices, ARM PL061 and TI Stellaris, which
share the same read/write functions (pl061_read and pl061_write).
However PL061 and Stellaris have different GPIO register definitions
and pl061_read()/pl061_write() doesn't check it. This patch enforces
checking on offset, preventing R/W into the reserved memory area.

Signed-off-by: Wei Huang <[e-mail]>
---
 hw/gpio/pl061.c | 30 ++++++++++++++++++++++--------
 1 file changed, 22 insertions(+), 8 deletions(-)

Comments

Peter Maydell Feb. 25, 2016, 12:59 p.m. UTC | #1

On 18 February 2016 at 16:56, Wei Huang <[e-mail]> wrote:
> pl061.c emulates two GPIO devices, ARM PL061 and TI Stellaris, which
> share the same read/write functions (pl061_read and pl061_write).
> However PL061 and Stellaris have different GPIO register definitions
> and pl061_read()/pl061_write() doesn't check it. This patch enforces
> checking on offset, preventing R/W into the reserved memory area.
>
> Signed-off-by: Wei Huang <[e-mail]>
> ---
>  hw/gpio/pl061.c | 30 ++++++++++++++++++++++--------
>  1 file changed, 22 insertions(+), 8 deletions(-)

Applied to target-arm.next, thanks.

-- PMM

Patch

diff --git a/hw/gpio/pl061.c b/hw/gpio/pl061.c
index e5a696e..013bd03 100644
--- a/hw/gpio/pl061.c
+++ b/hw/gpio/pl061.c
@@ -61,6 +61,7 @@  typedef struct PL061State {
     qemu_irq irq;
     qemu_irq out[8];
     const unsigned char *id;
+    uint32_t rsvd_start; /* reserved area: [rsvd_start, 0xfcc] */
 } PL061State;
 
 static const VMStateDescription vmstate_pl061 = {
@@ -154,12 +155,15 @@  static uint64_t pl061_read(void *opaque, hwaddr offset,
 {
     PL061State *s = (PL061State *)opaque;
 
-    if (offset >= 0xfd0 && offset < 0x1000) {
-        return s->id[(offset - 0xfd0) >> 2];
-    }
     if (offset < 0x400) {
         return s->data & (offset >> 2);
     }
+    if (offset >= s->rsvd_start && offset <= 0xfcc) {
+        goto err_out;
+    }
+    if (offset >= 0xfd0 && offset < 0x1000) {
+        return s->id[(offset - 0xfd0) >> 2];
+    }
     switch (offset) {
     case 0x400: /* Direction */
         return s->dir;
@@ -200,10 +204,12 @@  static uint64_t pl061_read(void *opaque, hwaddr offset,
     case 0x528: /* Analog mode select */
         return s->amsel;
     default:
-        qemu_log_mask(LOG_GUEST_ERROR,
-                      "pl061_read: Bad offset %x\n", (int)offset);
-        return 0;
+        break;
     }
+err_out:
+    qemu_log_mask(LOG_GUEST_ERROR,
+                  "pl061_read: Bad offset %x\n", (int)offset);
+    return 0;
 }
 
 static void pl061_write(void *opaque, hwaddr offset,
@@ -218,6 +224,9 @@  static void pl061_write(void *opaque, hwaddr offset,
         pl061_update(s);
         return;
     }
+    if (offset >= s->rsvd_start) {
+        goto err_out;
+    }
     switch (offset) {
     case 0x400: /* Direction */
         s->dir = value & 0xff;
@@ -276,10 +285,13 @@  static void pl061_write(void *opaque, hwaddr offset,
         s->amsel = value & 0xff;
         break;
     default:
-        qemu_log_mask(LOG_GUEST_ERROR,
-                      "pl061_write: Bad offset %x\n", (int)offset);
+        goto err_out;
     }
     pl061_update(s);
+    return;
+err_out:
+    qemu_log_mask(LOG_GUEST_ERROR,
+                  "pl061_write: Bad offset %x\n", (int)offset);
 }
 
 static void pl061_reset(PL061State *s)
@@ -327,6 +339,7 @@  static void pl061_luminary_init(Object *obj)
     PL061State *s = PL061(obj);
 
     s->id = pl061_id_luminary;
+    s->rsvd_start = 0x52c;
 }
 
 static void pl061_init(Object *obj)
@@ -334,6 +347,7 @@  static void pl061_init(Object *obj)
     PL061State *s = PL061(obj);
 
     s->id = pl061_id;
+    s->rsvd_start = 0x424;
 }
 
 static void pl061_class_init(ObjectClass *klass, void *data)