On Fri, Mar 22, 2019 at 01:35:57PM +0000, Peter Maydell wrote:
> On Fri, 22 Mar 2019 at 13:19, Dima Stepanov <dimas...@yandex-team.ru> 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 <dimas...@yandex-team.ru>
> > ---
> > 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().
Hard to say for me, if it is needed or not, since we have no xen
reproducer for this issue. Right now we are making some fuzzing for the
virtio-blk devices and hit these asserts which are good to fix.
>
> 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.
Maybe a fix at virtio level will be better in general, what do you
think?
Thanks, Dima.
>
> thanks
> -- PMM
