Re: [PATCH v4 9/9] memory: assert MemoryRegionOps callbacks are defined

[Philippe Mathieu-Daudé]

On 8/11/20 1:41 PM, P J P wrote:
> From: Prasad J Pandit <pjp@fedoraproject.org>
> 
> When registering a MemoryRegionOps object, assert that its
> read/write callback methods are defined. This avoids potential
> guest crash via a NULL pointer dereference.
> 
> Suggested-by: Peter Maydell <peter.maydell@linaro.org>
> Reviewed-by: Li Qiang <liq3ea@gmail.com>
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Prasad J Pandit <pjp@fedoraproject.org>

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> ---
>  softmmu/memory.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> Update v4: add Reviewed-by tag
>   -> https://lists.nongnu.org/archive/html/qemu-devel/2020-07/msg05324.html
> 
> diff --git a/softmmu/memory.c b/softmmu/memory.c
> index af25987518..1f4b37b3a6 100644
> --- a/softmmu/memory.c
> +++ b/softmmu/memory.c
> @@ -1485,7 +1485,13 @@ void memory_region_init_io(MemoryRegion *mr,
>                             uint64_t size)
>  {
>      memory_region_init(mr, owner, name, size);
> -    mr->ops = ops ? ops : &unassigned_mem_ops;
> +    if (ops) {
> +        assert(ops->read || ops->read_with_attrs);
> +        assert(ops->write || ops->write_with_attrs);
> +        mr->ops = ops;
> +    } else {
> +        mr->ops = &unassigned_mem_ops;
> +    }
>      mr->opaque = opaque;
>      mr->terminates = true;
>  }
> @@ -1663,6 +1669,8 @@ void 
> memory_region_init_rom_device_nomigrate(MemoryRegion *mr,
>  {
>      Error *err = NULL;
>      assert(ops);
> +    assert(ops->read || ops->read_with_attrs);
> +    assert(ops->write || ops->write_with_attrs);
>      memory_region_init(mr, owner, name, size);
>      mr->ops = ops;
>      mr->opaque = opaque;
>