From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:54157) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YVexg-0001cV-OO for qemu-devel@nongnu.org; Wed, 11 Mar 2015 07:38:05 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YVexb-00047w-Oo for qemu-devel@nongnu.org; Wed, 11 Mar 2015 07:38:04 -0400 Received: from ozlabs.org ([103.22.144.67]:38032) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YVexb-00047l-DS for qemu-devel@nongnu.org; Wed, 11 Mar 2015 07:37:59 -0400 From: Rusty Russell In-Reply-To: <20150311074938-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> Date: Wed, 11 Mar 2015 22:06:40 +1030 Message-ID: <87pp8fyddj.fsf@rustcorp.com.au> MIME-Version: 1.0 Content-Type: text/plain 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: "Michael S. Tsirkin" , Fam Zheng Cc: QEMU Developers , stefanha@redhat.com "Michael S. Tsirkin" writes: > On Wed, Mar 11, 2015 at 02:47:47PM +0800, Fam Zheng wrote: >> On Wed, 03/11 07:19, Michael S. Tsirkin wrote: >> > On Wed, Mar 11, 2015 at 04:29:30PM +1030, Rusty Russell wrote: >> > > The virtio 'used' ring describes descriptors which have been used. It >> > > also says how many bytes have been written to the ring. For some cases, >> > > this value is ignored by Linux guests, thus errors have not been noticed. >> > > I was working on increasing the checking in Linux when I noticed this >> > > behaviour. >> > > >> > > The first patch changes the 'len' formal parameter name to 'len_written' to >> > > make the API clearer, and adds an assert(). The second fixes block writes. >> > > >> > > Cheers, >> > > Rusty. >> > > PS. It's based on MST's virtio-1.0 tree, but should be easily ported. >> > >> > Thanks, this applies to current master without issues. >> > However, I think it's best to apply patch 2, then patch 1, >> > to avoid triggering errors when bisecting. >> >> I'm seeing a make check failure. If this is a false alarm, the test should be >> fixed too. > > Yea, I'm also now thinking we need a spec clarification on this one, and > some testing with non linux drivers before jumping to changing hosts and > guests. The spec is very clear. The implementation is crap; let's fix it before 1.0. Quote: 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. I have a patch for the Linux side, too, which warns once per device and fixes it up. I will make the warning conditional on v1.0. Cheers, Rusty.