On Mon, Mar 23, 2015 at 02:06:14PM +1030, Rusty Russell wrote:
> "Michael S. Tsirkin" <> writes:
> > On Fri, Mar 20, 2015 at 11:48:55AM +1030, Rusty Russell wrote:
> >> We said what it was for, and noted why. We didn't place any requirements
> >> on it, nor clearly spell out the implications of its use.
> >>
> >> This clarification comes particularly from noticing that QEMU didn't
> >> set len correctly, and philosophising over the correct value when
> >> an error has occurred.
> >>
> >> (Wording precision feedback from Michael and Cornelia - Thanks!)
> >>
> >> Signed-off-by: Rusty Russell <>
> >
> > Regarding 1.0 versus 1.1: do you think this is a non-material change?
> > If we do material changes, we need a public review period,
> > and I feel reviewer's time is better spent reviewing 1.1.
>
> It's borderline. These requirements are implied by the text but not
> spelled out. Clearly it is better to do so, but I don't think it's
> worth a full review period.
So, if you think we should include this in 1.0 cs03, then you
need to vote no on the cs03 ballot, and we'll
redo a draft with these changes applied.
> >> diff --git a/content.tex b/content.tex
> >> index 6ba079d..2ff8c65 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
> >
> > I would add "in particular" here. It's also useful for many other drivers.
>
> OK, I said "particularly 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 could leak the
> >> +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}
> >
> > Notification Suppression?
>
> Hmm, cut & paste bug.
>
> >> +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} is
> >> +permitted to be an underestimate: 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}
> >
> >
> > Same here.
> >
> >> +
> >> +The driver MUST NOT make assumptions about data in device-writable buffers
> >> +beyond the first \field{len} bytes, and SHOULD ignore this data.
> >> +
> >> \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
>
> Inter-diff:
>
> diff --git a/content.tex b/content.tex
> index 2ff8c65..8e8765d 100644
> --- a/content.tex
> +++ b/content.tex
> @@ -603,7 +603,7 @@ placed in the available ring by the guest earlier), and \field{len} the total
> of bytes written into the buffer.
>
> \begin{note}
> -\field{len} is useful
> +\field{len} is particularly 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.
> @@ -621,7 +621,7 @@ 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}
> +\devicenormative{\subsubsection}{The Virtqueue Used Ring}{Basic Facilities of a Virtio Device / Virtqueues / The Virtqueue Used Ring}
>
> The device MUST set \field{len} prior to updating the used \field{idx}.
>
> @@ -638,7 +638,7 @@ permitted to be an underestimate: 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}
> +\drivernormative{\subsubsection}{The Virtqueue Used Ring}{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 this data.
