[PULL,21/21] system/physmem: Where we assume we have a RAM MR, assert it

Commit Message

Peter Maydell July 30, 2024, 9:40 a.m. UTC

In the functions invalidate_and_set_dirty() and
cpu_physical_memory_snapshot_and_clear_dirty(), we assume that we
are dealing with RAM memory regions. In this case we know that
memory_region_get_ram_addr() will succeed. Assert this before we
use the returned ram_addr_t in arithmetic.

This makes Coverity happier about these functions: it otherwise
complains that we might have an arithmetic overflow that stems
from the possible -1 return from memory_region_get_ram_addr().

Resolves: Coverity CID 1547629, 1547715

Signed-off-by: Peter Maydell <[e-mail]>
Reviewed-by: Peter Xu <[e-mail]>
Reviewed-by: David Hildenbrand <[e-mail]>
Message-id: [e-mail]
---
 system/physmem.c | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

Patch

diff --git a/system/physmem.c b/system/physmem.c
index 0e19186e1b4..94600a33ec3 100644
--- a/system/physmem.c
+++ b/system/physmem.c
@@ -923,13 +923,19 @@  DirtyBitmapSnapshot *cpu_physical_memory_snapshot_and_clear_dirty
     (MemoryRegion *mr, hwaddr offset, hwaddr length, unsigned client)
 {
     DirtyMemoryBlocks *blocks;
-    ram_addr_t start = memory_region_get_ram_addr(mr) + offset;
+    ram_addr_t start, first, last;
     unsigned long align = 1UL << (TARGET_PAGE_BITS + BITS_PER_LEVEL);
-    ram_addr_t first = QEMU_ALIGN_DOWN(start, align);
-    ram_addr_t last  = QEMU_ALIGN_UP(start + length, align);
     DirtyBitmapSnapshot *snap;
     unsigned long page, end, dest;
 
+    start = memory_region_get_ram_addr(mr);
+    /* We know we're only called for RAM MemoryRegions */
+    assert(start != RAM_ADDR_INVALID);
+    start += offset;
+
+    first = QEMU_ALIGN_DOWN(start, align);
+    last  = QEMU_ALIGN_UP(start + length, align);
+
     snap = g_malloc0(sizeof(*snap) +
                      ((last - first) >> (TARGET_PAGE_BITS + 3)));
     snap->start = first;
@@ -2659,7 +2665,11 @@  static void invalidate_and_set_dirty(MemoryRegion *mr, hwaddr addr,
                                      hwaddr length)
 {
     uint8_t dirty_log_mask = memory_region_get_dirty_log_mask(mr);
-    addr += memory_region_get_ram_addr(mr);
+    ram_addr_t ramaddr = memory_region_get_ram_addr(mr);
+
+    /* We know we're only called for RAM MemoryRegions */
+    assert(ramaddr != RAM_ADDR_INVALID);
+    addr += ramaddr;
 
     /* No early return if dirty_log_mask is or becomes 0, because
      * cpu_physical_memory_set_dirty_range will still call