Re: [Qemu-devel] [PATCH 0/2] virtio len fixes for qemu.

[Cornelia Huck]

On Mon, 16 Mar 2015 06:03:24 +0100
"Michael S. Tsirkin" <address@hidden> wrote:

> On Mon, Mar 16, 2015 at 01:44:22PM +1030, Rusty Russell wrote:

> > diff --git a/content.tex b/content.tex
> > index 6ba079d..2c946a5 100644
> > --- a/content.tex
> > +++ b/content.tex
> > @@ -600,10 +600,19 @@ them: it is only written to by the device, and read 
> > by the driver.
> >  Each entry in the ring is a pair: \field{id} indicates the head entry of 
> > the
> >  descriptor chain describing the buffer (this matches an entry
> >  placed in the available ring by the guest earlier), and \field{len} the 
> > total
> > -of bytes written into the buffer. The latter is extremely useful
> > -for drivers using untrusted buffers: if you do not know exactly
> > -how much has been written by the device, you usually have to zero
> > -the buffer to ensure no data leakage occurs.
> > +of bytes written into the buffer. 
> > +
> > +\begin{note}
> > +\field{len} is useful
> > +for drivers using untrusted buffers: if a driver does not know exactly
> > +how much has been written by the device, the driver would have to zero
> > +the buffer in advance to ensure no data leakage occurs.
> > +
> > +For example, a network driver may hand a received buffer directly to
> > +an unprivileged userspace application.  If the network device has not
> > +overwritten the bytes which were in that buffer, this may leak the

s/may/might/ in both cases, so it doesn't get confused with MAY?

> > +contents of freed memory from other processes to the application.
> > +\end{note}
> >  
> >  \begin{note}
> >  The legacy \hyperref[intro:Virtio PCI Draft]{[Virtio PCI Draft]}
> > @@ -612,6 +621,28 @@ the constant as VRING_USED_F_NO_NOTIFY, but the layout 
> > and value were
> >  identical.
> >  \end{note}
> >  
> > +\devicenormative{\subsubsection}{Virtqueue Notification Suppression}{Basic 
> > Facilities of a Virtio Device / Virtqueues / The Virtqueue Used Ring}
> > +
> > +The device MUST set \field{len} prior to updating the used \field{idx}.
> > +
> > +The device MUST write at least \field{len} bytes to descriptor,
> > +beginning at the first device-writable buffer,
> > +prior to updating the used \field{idx}.
> > +
> > +The device MAY write more than \field{len} bytes to descriptor.
> > +
> > +\begin{note}
> > +There are potential error cases where a device might not know what
> > +parts of the buffers have been written.  This is why \field{len} may
> > +be an underestimate, but that's preferable to the driver believing
> > +that uninitialized memory has been overwritten when it has not.
> > +\end{note}
> > +
> > +\drivernormative{\subsubsection}{Virtqueue Notification Suppression}{Basic 
> > Facilities of a Virtio Device / Virtqueues / The Virtqueue Used Ring}
> > +
> > +The driver MUST NOT make assumptions about data in device-writable buffers
> > +beyond the first \field{len} bytes, and SHOULD ignore it.
> 
> it -> this data.
> 
> Otherwise on first reading I thought "it" refers to len field.
> 
> > +
> >  \subsection{Virtqueue Notification Suppression}\label{sec:Basic Facilities 
> > of a Virtio Device / Virtqueues / Virtqueue Notification Suppression}
> >  
> >  The device can suppress notifications in a manner analogous to the way
> 
> Sounds good, let's move discussion to virtio/virtio-dev now?

FWIW, this sounds good to me as well.

> I think it's 1.1 material - agree?

Given that nobody seems to have cared about this before, I agree.