On 8/18/20 11:52 AM, P J P wrote:
> +-- On Mon, 17 Aug 2020, Philippe Mathieu-Daudé wrote --+
> | Fix by initializing the MemoryRegionOps to ram_device_mem_ops, this way the
> | memory accesses are properly dispatched using
> | memory_region_ram_device_read() / memory_region_ram_device_write().
> |
> | Fixes: 4a2e242bbb ("memory: Don't use memcpy for ram_device regions")
> | Signed-off-by: Philippe Mathieu-Daudé <f4...@amsat.org>
> | ---
> | Since v1: Corrected description (PJP)
> | Cc: P J P <ppan...@redhat.com>
> | ---
> | softmmu/memory.c | 10 ++++++++++
> | 1 file changed, 10 insertions(+)
> |
> | diff --git a/softmmu/memory.c b/softmmu/memory.c
> | index 651705b7d1..8139da1a58 100644
> | --- a/softmmu/memory.c
> | +++ b/softmmu/memory.c
> | @@ -1517,6 +1517,8 @@ void
> memory_region_init_ram_shared_nomigrate(MemoryRegion *mr,
> | Error *err = NULL;
> | memory_region_init(mr, owner, name, size);
> | mr->ram = true;
> | + mr->ops = &ram_device_mem_ops;
> | + mr->opaque = mr;
> | mr->terminates = true;
>
>
> I wonder if 'mr->ops = &ram_device_mem_ops' could be done in
> memory_region_init() instead?
I don't think so, because to use &ram_device_mem_ops handlers use
mr->opaque and mr->ram_block, so alias/io/iommu/rom types would
directly crash accessing these NULL fields.
>
> Otherwise looks okay.
Thanks!
>
> Thank you.
> --
> Prasad J Pandit / Red Hat Product Security Team
> 8685 545E B54C 486B C6EB 271E E285 8B5A F050 DE8D
>
