On Fri, Mar 13, 2015 at 11:47:18AM +1030, Rusty Russell wrote: > "Michael S. Tsirkin" <m...@redhat.com> writes: > > On Thu, Mar 12, 2015 at 11:34:35AM +1030, Rusty Russell wrote: > >> "Michael S. Tsirkin" <m...@redhat.com> writes: > >> > On Wed, Mar 11, 2015 at 10:06:40PM +1030, Rusty Russell wrote: > >> >> 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. > >> > > >> > Right so what does this "if you do not know exactly how much has been > >> > written by the device" mean? > >> > >> It means "without this feature, you would not know how much has been > >> written by the device"... > > > > So imagine a situation where device does not know for sure > > how much was written, like here. > > Should it set len to value that was written for sure? > > Or to value that was possibly written? > > In this particular case, it doesn't matter since the failure is marked. > > In general, as the stated purpose of 'len' is to avoid guest > receive-buffer zeroing, it is implied that it must not overestimate. > > Imagine the case of a guest user process receiving network packets. If > the net device says it's written 1000 bytes (but it hasn't) we will hand > 1000 bytes of uninitialized kernel memory to that process.
Finally, I think I understand. Thanks for your patience. > Here's my proposed spec patch, which spells this out: > > diff --git a/content.tex b/content.tex > index 6ba079d..b6345a8 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 > +of bytes written into the buffer. > + > +\begin{note} > +\field{len} is extremely useful just "useful" maybe? > for drivers using untrusted buffers: if you do not know exactly replace "you" with "driver" here? > -how much has been written by the device, you usually have to zero > -the buffer to ensure no data leakage occurs. > +how much has been written by the device, a driver would have to zero > +the buffer in advance to ensure no data leakage occurs. > + > +For example, a network driver any driver really, right? > 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 > +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,19 @@ 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} to the number of bytes known to be > +written to the descriptor, beginning at the first device-writable > +buffer. I think "known to be written" is still too indeterministic for my taste. Reminds me of the Schrödinger's cat experiment for some reason. How about something like this: +The device MUST write at least \field{len} bytes to descriptor, +beginning at the first device-writable buffer, +prior to updating the used index field. +The device MAY write more than \field{len} bytes to descriptor. +The driver MUST NOT make assumptions about data in the buffer pointed to +by the descriptor with WRITE flag +beyond the first \field{len} bytes: the data +might be unchanged by the device, or it might be +overwritten by the device. +The driver SHOULD ignore data beyond the first \field{len} bytes. > + > +\begin{note} > +There are potential error cases where a device might not know what > +parts of the buffers have been written. In this case \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} > + > \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