[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]
Subject: Re: [PATCH] used ring: define the meaning and requirements of the len field.
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. I'm having second thoughts about this philosophising here :) Imagine this driver with untrusted buffers used by multiple applications. Assume application 1 uses the device. We get len value from device but with your change applied, we don't really know whether any bytes > len have been modified. Passing them to application 2 will thus leak some info from application 1 - thus driver must zero out the rest of buffer before reuse. To me, it looks like - existing spec text can be understood to match QEMU behaviour - it isn't obviously clear what's best Thoughts? > (Wording precision feedback from Michael and Cornelia - Thanks!) > > Signed-off-by: Rusty Russell <rusty@rustcorp.com.au> > > 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 > +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} > + > +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} > + > +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
[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]