| Message ID | 20240723170513.1676453-1-peter.maydell@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | system/physmem: Where we assume we have a RAM MR, assert it | expand |
On Tue, Jul 23, 2024 at 06:05:13PM +0100, Peter Maydell wrote: > 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 <peter.maydell@linaro.org> Reviewed-by: Peter Xu <peterx@redhat.com>
On 23.07.24 19:05, Peter Maydell wrote: > 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 <peter.maydell@linaro.org> > --- Reviewed-by: David Hildenbrand <david@redhat.com>
On Tue, 23 Jul 2024 at 18:05, Peter Maydell <peter.maydell@linaro.org> wrote: > > 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 <peter.maydell@linaro.org> > -- I'm doing a target-arm pullreq so I'll take this patch through that, unless you'd prefer otherwise. thanks -- PMM
diff --git a/system/physmem.c b/system/physmem.c index 9a3b3a76360..87554d68ea4 100644 --- a/system/physmem.c +++ b/system/physmem.c @@ -894,13 +894,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; @@ -2630,7 +2636,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
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 <peter.maydell@linaro.org> --- system/physmem.c | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-)