[Qemu-devel] [PATCH v2] highbank: validate register offset before access

P J P posted 1 patch 6 years, 4 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20171113062658.9697-1-ppandit@redhat.com
Test checkpatch passed
Test docker passed
Test ppc passed
Test s390x passed
hw/arm/highbank.c | 17 +++++++++++++++--
1 file changed, 15 insertions(+), 2 deletions(-)
[Qemu-devel] [PATCH v2] highbank: validate register offset before access
Posted by P J P 6 years, 4 months ago
From: Prasad J Pandit <pjp@fedoraproject.org>

An 'offset' parameter sent to highbank register r/w functions
could be greater than number(NUM_REGS=0x200) of hb registers,
leading to an OOB access issue. Add check to avoid it.

Reported-by: Moguofang (Dennis mo) <moguofang@huawei.com>
Signed-off-by: Prasad J Pandit <pjp@fedoraproject.org>
---
 hw/arm/highbank.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

Update: use HWADDR_PRIx to print offset
  -> https://lists.gnu.org/archive/html/qemu-devel/2017-11/msg02116.html

diff --git a/hw/arm/highbank.c b/hw/arm/highbank.c
index 354c6b25a8..287392bbdc 100644
--- a/hw/arm/highbank.c
+++ b/hw/arm/highbank.c
@@ -34,6 +34,7 @@
 #include "hw/ide/ahci.h"
 #include "hw/cpu/a9mpcore.h"
 #include "hw/cpu/a15mpcore.h"
+#include "qemu/log.h"
 
 #define SMP_BOOT_ADDR           0x100
 #define SMP_BOOT_REG            0x40
@@ -117,14 +118,26 @@ static void hb_regs_write(void *opaque, hwaddr offset,
         }
     }
 
-    regs[offset/4] = value;
+    if (offset / 4 >= NUM_REGS) {
+        qemu_log_mask(LOG_GUEST_ERROR,
+                  "highbank: bad write offset 0x%" HWADDR_PRIx "\n", offset);
+        return;
+    }
+    regs[offset / 4] = value;
 }
 
 static uint64_t hb_regs_read(void *opaque, hwaddr offset,
                              unsigned size)
 {
+    uint32_t value;
     uint32_t *regs = opaque;
-    uint32_t value = regs[offset/4];
+
+    if (offset / 4 >= NUM_REGS) {
+        qemu_log_mask(LOG_GUEST_ERROR,
+                  "highbank: bad read offset 0x%" HWADDR_PRIx "\n", offset);
+        return 0;
+    }
+    value = regs[offset / 4];
 
     if ((offset == 0x100) || (offset == 0x108) || (offset == 0x10C)) {
         value |= 0x30000000;
-- 
2.13.6


Re: [Qemu-devel] [PATCH v2] highbank: validate register offset before access
Posted by Peter Maydell 6 years, 4 months ago
On 13 November 2017 at 06:26, P J P <ppandit@redhat.com> wrote:
> From: Prasad J Pandit <pjp@fedoraproject.org>
>
> An 'offset' parameter sent to highbank register r/w functions
> could be greater than number(NUM_REGS=0x200) of hb registers,
> leading to an OOB access issue. Add check to avoid it.
>
> Reported-by: Moguofang (Dennis mo) <moguofang@huawei.com>
> Signed-off-by: Prasad J Pandit <pjp@fedoraproject.org>
> ---
>  hw/arm/highbank.c | 17 +++++++++++++++--
>  1 file changed, 15 insertions(+), 2 deletions(-)
>
> Update: use HWADDR_PRIx to print offset
>   -> https://lists.gnu.org/archive/html/qemu-devel/2017-11/msg02116.html



Applied to target-arm.next, thanks.

-- PMM