Re: [Qemu-devel] [2.2 PATCH V2 for-4.5] virtio-net: fix unmap leak

[Konrad Rzeszutek Wilk]

On Nov 27, 2014 9:58 AM, Stefano Stabellini <address@hidden> wrote:
>
> On Thu, 27 Nov 2014, Konrad Rzeszutek Wilk wrote: 
> > On Nov 27, 2014 7:46 AM, Stefano Stabellini <address@hidden> wrote: 
> > > 
> > > Konrad, I think we should have this fix in 4.5: without it 
> > > vif=[ 'model=virtio-net' ] crashes QEMU. 
> > > 
> > 
> > Is it an regression? 
>
> Good question: I was trying to investigate that. 
>
> virtio-net is currently *not* documented in the xl interface: 
>
>
> ### model 
>
> This keyword is valid for HVM guest devices with `type=ioemu` only. 
>
> Specifies the type device to emulated for this guest. Valid values 
> are: 
>
>   * `rtl8139` (default) -- Realtek RTL8139 
>   * `e1000` -- Intel E1000 
>   * in principle any device supported by your device model 
>
>
> The last working version of virtio-net on Xen is QEMU v1.4.0. That means 
> that the bug affects Xen 4.4 too (but it should work in Xen 4.3). 

Not a regression compared to 4.4 but it has been for two releases.

So if nobody noticed it for two releases will they notice it if it not fixed in 
this release either? And can it be fixed in the next one?


>
>
> > > On Thu, 27 Nov 2014, Peter Maydell wrote: 
> > > > On 27 November 2014 at 12:33, Michael S. Tsirkin <address@hidden> 
> > > > wrote: 
> > > > > On Thu, Nov 27, 2014 at 06:04:03PM +0800, Jason Wang wrote: 
> > > > >> virtio_net_handle_ctrl() and other functions that process control vq 
> > > > >> request call iov_discard_front() which will shorten the iov. This 
> > > > >> will 
> > > > >> lead unmapping in virtqueue_push() leaks mapping. 
> > > > >> 
> > > > >> Fixes this by keeping the original iov untouched and using a temp 
> > > > >> variable 
> > > > >> in those functions. 
> > > > >> 
> > > > >> Cc: Wen Congyang <address@hidden> 
> > > > >> Cc: Stefano Stabellini <address@hidden> 
> > > > >> Cc: address@hidden 
> > > > >> Signed-off-by: Jason Wang <address@hidden> 
> > > > > 
> > > > > Reviewed-by: Michael S. Tsirkin <address@hidden> 
> > > > > 
> > > > > Peter, can you pick this up or do you want a pull request? 
> > > > 
> > > > I can pick it up. I was waiting a bit to check that everybody 
> > > > was happy that this is the correct way to fix the bug and the 
> > > > patch is ok... 
> >