[PATCH] hw/net/npcm_gmac: Catch accesses off the end of the register array

Peter Maydell posted 1 patch 1 month, 1 week ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20260306154016.2194091-1-peter.maydell@linaro.org
Maintainers: Tyrone Ting <kfting@nuvoton.com>, Hao Wu <wuhaotsh@google.com>, Jason Wang <jasowang@redhat.com>
hw/net/npcm_gmac.c         | 14 ++++++++++++++
include/hw/net/npcm_gmac.h |  3 ++-
2 files changed, 16 insertions(+), 1 deletion(-)
[PATCH] hw/net/npcm_gmac: Catch accesses off the end of the register array
Posted by Peter Maydell 1 month, 1 week ago
In the npcm_gmac device, we create the iomem MemoryRegion with
a size of 8KB, but NPCM_GMAC_NR_REGS is only 0x1060 / 4. This
means there's a range of offsets that the guest can access
that don't have gmac->regs[] entries. We weren't catching this,
so the guest could get us to index off the end of the regs array.

Catch and log these invalid accesses.

Cc: qemu-stable@nongnu.org
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/3316
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/net/npcm_gmac.c         | 14 ++++++++++++++
 include/hw/net/npcm_gmac.h |  3 ++-
 2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/hw/net/npcm_gmac.c b/hw/net/npcm_gmac.c
index 123fb92ca4..d9902d9ab5 100644
--- a/hw/net/npcm_gmac.c
+++ b/hw/net/npcm_gmac.c
@@ -700,6 +700,13 @@ static uint64_t npcm_gmac_read(void *opaque, hwaddr offset, unsigned size)
     NPCMGMACState *gmac = opaque;
     uint32_t v = 0;
 
+    if (offset >= NPCM_GMAC_REG_SIZE) {
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "%s: invalid register offset: 0x%04" HWADDR_PRIx"\n",
+                      DEVICE(gmac)->canonical_path, offset);
+        return v;
+    }
+
     switch (offset) {
     /* Write only registers */
     case A_NPCM_DMA_XMT_POLL_DEMAND:
@@ -724,6 +731,13 @@ static void npcm_gmac_write(void *opaque, hwaddr offset,
 
     trace_npcm_gmac_reg_write(DEVICE(gmac)->canonical_path, offset, v);
 
+    if (offset >= NPCM_GMAC_REG_SIZE) {
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "%s: invalid register offset: 0x%04" HWADDR_PRIx"\n",
+                      DEVICE(gmac)->canonical_path, offset);
+        return;
+    }
+
     switch (offset) {
     /* Read only registers */
     case A_NPCM_GMAC_VERSION:
diff --git a/include/hw/net/npcm_gmac.h b/include/hw/net/npcm_gmac.h
index d4fe49ada5..23b9841a80 100644
--- a/include/hw/net/npcm_gmac.h
+++ b/include/hw/net/npcm_gmac.h
@@ -24,7 +24,8 @@
 #include "hw/core/sysbus.h"
 #include "net/net.h"
 
-#define NPCM_GMAC_NR_REGS (0x1060 / sizeof(uint32_t))
+#define NPCM_GMAC_REG_SIZE 0x1060
+#define NPCM_GMAC_NR_REGS (NPCM_GMAC_REG_SIZE / sizeof(uint32_t))
 
 #define NPCM_GMAC_MAX_PHYS 32
 #define NPCM_GMAC_MAX_PHY_REGS 32
-- 
2.43.0
Re: [PATCH] hw/net/npcm_gmac: Catch accesses off the end of the register array
Posted by Philippe Mathieu-Daudé 1 month ago
On 6/3/26 16:40, Peter Maydell wrote:
> In the npcm_gmac device, we create the iomem MemoryRegion with
> a size of 8KB, but NPCM_GMAC_NR_REGS is only 0x1060 / 4. This
> means there's a range of offsets that the guest can access
> that don't have gmac->regs[] entries. We weren't catching this,
> so the guest could get us to index off the end of the regs array.
> 
> Catch and log these invalid accesses.
> 
> Cc: qemu-stable@nongnu.org
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/3316
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   hw/net/npcm_gmac.c         | 14 ++++++++++++++
>   include/hw/net/npcm_gmac.h |  3 ++-
>   2 files changed, 16 insertions(+), 1 deletion(-)

Queued via hw-misc, thanks.
Re: [PATCH] hw/net/npcm_gmac: Catch accesses off the end of the register array
Posted by Philippe Mathieu-Daudé 1 month ago
On 6/3/26 16:40, Peter Maydell wrote:
> In the npcm_gmac device, we create the iomem MemoryRegion with
> a size of 8KB, but NPCM_GMAC_NR_REGS is only 0x1060 / 4. This
> means there's a range of offsets that the guest can access
> that don't have gmac->regs[] entries. We weren't catching this,
> so the guest could get us to index off the end of the regs array.
> 
> Catch and log these invalid accesses.
> 
> Cc: qemu-stable@nongnu.org
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/3316
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   hw/net/npcm_gmac.c         | 14 ++++++++++++++
>   include/hw/net/npcm_gmac.h |  3 ++-
>   2 files changed, 16 insertions(+), 1 deletion(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

and queued.