From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:50319) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YWPy5-0004kb-9l for qemu-devel@nongnu.org; Fri, 13 Mar 2015 09:49:38 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YWPy2-0006jT-DH for qemu-devel@nongnu.org; Fri, 13 Mar 2015 09:49:37 -0400 Received: from mx1.redhat.com ([209.132.183.28]:51826) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YWPy2-0006j6-5B for qemu-devel@nongnu.org; Fri, 13 Mar 2015 09:49:34 -0400 Date: Fri, 13 Mar 2015 14:49:28 +0100 From: "Michael S. Tsirkin" Message-ID: <20150313143624-mutt-send-email-mst@redhat.com> References: <1426053572-21326-1-git-send-email-rusty@rustcorp.com.au> <20150311061935.GA13037@redhat.com> <20150311064747.GF1437@ad.nay.redhat.com> <20150311074938-mutt-send-email-mst@redhat.com> <87pp8fyddj.fsf@rustcorp.com.au> <20150311133242-mutt-send-email-mst@redhat.com> <87bnjzxbz0.fsf@rustcorp.com.au> <20150312073038-mutt-send-email-mst@redhat.com> <87bnjxwva9.fsf@rustcorp.com.au> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline In-Reply-To: <87bnjxwva9.fsf@rustcorp.com.au> Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH 0/2] virtio len fixes for qemu. List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Rusty Russell Cc: Fam Zheng , QEMU Developers , stefanha@redhat.com On Fri, Mar 13, 2015 at 11:47:18AM +1030, Rusty Russell wrote: > "Michael S. Tsirkin" writes: > > On Thu, Mar 12, 2015 at 11:34:35AM +1030, Rusty Russell wrote: > >> "Michael S. Tsirkin" 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 th= e head > >> >> entry of the descriptor chain describing the buffer (this > >> >> matches an entry placed in the available ring by the gues= t > >> >> 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 ha= s been > >> >> written by the device, you usually have to zero the buffe= r to > >> >> ensure no data leakage occurs. > >> > > >> > Right so what does this "if you do not know exactly how much has b= een > >> > written by the device" mean? > >>=20 > >> 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? >=20 > In this particular case, it doesn't matter since the failure is marked. >=20 > In general, as the stated purpose of 'len' is to avoid guest > receive-buffer zeroing, it is implied that it must not overestimate. >=20 > 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 han= d > 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: >=20 > 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 re= ad 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} th= e total > -of bytes written into the buffer. The latter is extremely useful > +of bytes written into the buffer.=20 > + > +\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} > =20 > \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 la= yout and value were > identical. > \end{note} > =20 > +\devicenormative{\subsubsection}{Virtqueue Notification Suppression}{B= asic 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=F6dinger'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 Facili= ties of a Virtio Device / Virtqueues / Virtqueue Notification Suppression= } > =20 > The device can suppress notifications in a manner analogous to the way