Re: [Qemu-devel] [PATCH v2] virtio: guard vring access when setting notification

[Cornelia Huck]

On Wed, 1 Mar 2017 19:07:14 +0200
"Michael S. Tsirkin" <address@hidden> wrote:

> On Wed, Mar 01, 2017 at 05:56:56PM +0100, Cornelia Huck wrote:
> > Switching to vring caches exposed an existing bug in
> > virtio_queue_set_notification(): We can't access vring structures
> > if they have not been set up yet. This may happen, for example,
> > for virtio-blk devices with multiple queues: The code will try to
> > switch notifiers for every queue, but the guest may have only set up
> > a subset of them.
> > 
> > Fix this by (1) guarding access to the vring memory by checking
> > for vring.desc and (2) triggering an update to the vring flags
> > for consistency with the configured notification state once the
> > queue is actually configured AND the device is in a state that
> > the rings may be actually accessed (i.e. DRIVER_OK has been set
> > or a legacy device kicks for the first time).
> 
> I am still puzzled about 2. drivers pre-zero rings as they
> are set up so notification is on by default. Why do you want to poke
> at it one extra time? You might get an extra notification
> if you tried to disable too early but is this really so bad?

Just checking for !desc and then leaving the queues alone might be the
right thing. See also my reply in the other thread.