From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753002AbcGTLcD (ORCPT ); Wed, 20 Jul 2016 07:32:03 -0400 Received: from mail-wm0-f68.google.com ([74.125.82.68]:35210 "EHLO mail-wm0-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751853AbcGTLb7 (ORCPT ); Wed, 20 Jul 2016 07:31:59 -0400 Date: Wed, 20 Jul 2016 09:21:08 +0100 From: Stefan Hajnoczi To: Namhyung Kim Cc: LKML , Paolo Bonzini , Radim Kr??m???? , "Michael S. Tsirkin" , Anthony Liguori , Anton Vorontsov , Colin Cross , Kees Cook , Tony Luck , Steven Rostedt , Ingo Molnar , Minchan Kim , kvm@vger.kernel.org, qemu-devel@nongnu.org, virtualization@lists.linux-foundation.org Subject: Re: [PATCH 2/3] qemu: Implement virtio-pstore device Message-ID: <20160720082108.GA13233@stefanha-x1.localdomain> References: <1468816661-6345-1-git-send-email-namhyung@kernel.org> <1468816661-6345-3-git-send-email-namhyung@kernel.org> <20160718100353.GA15163@stefanha-x1.localdomain> <20160719154839.GB20047@danjae.aot.lge.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="qDbXVdCdHGoSgWSk" Content-Disposition: inline In-Reply-To: <20160719154839.GB20047@danjae.aot.lge.com> User-Agent: Mutt/1.6.1 (2016-04-27) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org --qDbXVdCdHGoSgWSk Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Jul 20, 2016 at 12:48:39AM +0900, Namhyung Kim wrote: > Hello, >=20 > On Mon, Jul 18, 2016 at 11:03:53AM +0100, Stefan Hajnoczi wrote: > > On Mon, Jul 18, 2016 at 01:37:40PM +0900, Namhyung Kim wrote: > > > +static void virtio_pstore_handle_io(VirtIODevice *vdev, VirtQueue *v= q) > > > +{ > > > + VirtIOPstore *s =3D VIRTIO_PSTORE(vdev); > > > + VirtQueueElement *elem; > > > + struct virtio_pstore_hdr *hdr; > > > + ssize_t len; > > > + > > > + for (;;) { > > > + elem =3D virtqueue_pop(vq, sizeof(VirtQueueElement)); > > > + if (!elem) { > > > + return; > > > + } > > > + > > > + hdr =3D elem->out_sg[0].iov_base; > > > + if (elem->out_sg[0].iov_len !=3D sizeof(*hdr)) { > > > + error_report("invalid header size: %u", > > > + (unsigned)elem->out_sg[0].iov_len); > > > + exit(1); > > > + } > >=20 > > Please use iov_to_buf() instead of directly accessing out_sg[]. Virtio > > devices are not supposed to assume a particular iovec layout. In other > > words, virtio_pstore_hdr could be split across multiple out_sg[] iovecs. > >=20 > > You must also copy in data (similar to Linux syscall implementations) to > > prevent the guest from modifying data while the command is processed. > > Such race conditions could lead to security bugs. >=20 > By accessing elem->out_sg[0].iov_base directly, I abused it as an > in-and-out buffer. But it seems not allowed by the virtio spec, do I > have to use separate buffers for request and response? Yes, a virtqueue element has (host read-only) out buffers followed by (host write-only) in buffers. Stefan --qDbXVdCdHGoSgWSk Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQEcBAEBAgAGBQJXjzR0AAoJEJykq7OBq3PIRZYH/A/fC1OAZbkPmx3q73/kipmD FnIsKLz4TejkZngO4OusmP/Ct3t1ydUJKVkkh6dbMKggxRgOB4y3m4AbZ17Xjdz+ OCSmqhKyoO+t3dpfuxysOx4Nk7WDdKs1Cdm8sTRUHcOyeeUh7ae1AOgPU+4VmczZ JK6Fok+HnUCszCp49D3EKkw8F3kqe3+uXc3x+bVS1qZWlEZHutGTJ0UoTXzdPItl H2LV/uzSB9EXuzdsfmPwU0fetXQFoFrvKsexqmIsyD5ORvhgjg3OOIabuozCLhwi 6yF1AZByLmBB1b961yXvf4x1Jnw3kfiJlklthfHw+vG1y+MVtKA5PoSnLHTawvM= =LvQu -----END PGP SIGNATURE----- --qDbXVdCdHGoSgWSk--