Re: [Qemu-devel] [PATCH v1] exec: check the range in the address_space_unmap routine

[Peter Maydell]

On Fri, 22 Mar 2019 at 13:19, Dima Stepanov <address@hidden> wrote:
>
> In case of the virtio-blk communication, can get the following assertion
> for the specifically crafted virtio packet:
>   qemu-system-x86_64: exec.c:3725: address_space_unmap: Assertion `mr !=
>   NULL' failed.
> This assertion is triggered if the length of the first descriptor in the
> block request chain (block command descriptor) is more than block command
> size. In this case the hw/block/virtio-blk.c:virtio_blk_handle_request()
> routine calls the iov_discard_front() function and the iov base and size
> are changed. As a result the address can not be found during the
> address_space_unmap() call.
>
> The fix is to check the whole address range in the address_space_unmap
> function.
>
> Signed-off-by: Dima Stepanov <address@hidden>
> ---
>  exec.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/exec.c b/exec.c
> index 86a38d3..0eeb018 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -3717,7 +3717,7 @@ void *address_space_map(AddressSpace *as,
>  void address_space_unmap(AddressSpace *as, void *buffer, hwaddr len,
>                           int is_write, hwaddr access_len)
>  {
> -    if (buffer != bounce.buffer) {
> +    if ((buffer < bounce.buffer) || (buffer + access_len > bounce.buffer + 
> bounce.len)) {
>          MemoryRegion *mr;
>          ram_addr_t addr1;

A quick look at the xen_invalidate_map_cache_entry() implementation
suggests that it also assumes that the address passed to
address_space_unmap() must be the same address that was
originally handed out via address_space_map().

So I think we either need to also change the Xen code, or
we need to fix this at the virtio level by having it keep
track of the buffer it was handed so it can unmap it.

thanks
-- PMM