From nobody Thu May 2 05:48:00 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) client-ip=208.118.235.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; Authentication-Results: mx.zoho.com; spf=pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org; Return-Path: Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) by mx.zohomail.com with SMTPS id 1498683094527720.0980991696179; Wed, 28 Jun 2017 13:51:34 -0700 (PDT) Received: from localhost ([::1]:35611 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dQJvx-0004fn-3F for importer@patchew.org; Wed, 28 Jun 2017 16:51:33 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40163) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dQJoz-0006tS-4S for qemu-devel@nongnu.org; Wed, 28 Jun 2017 16:44:22 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1dQJow-000425-2V for qemu-devel@nongnu.org; Wed, 28 Jun 2017 16:44:21 -0400 Received: from 20.mo5.mail-out.ovh.net ([91.121.55.239]:59357) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1dQJov-00041V-Ra for qemu-devel@nongnu.org; Wed, 28 Jun 2017 16:44:17 -0400 Received: from player799.ha.ovh.net (b9.ovh.net [213.186.33.59]) by mo5.mail-out.ovh.net (Postfix) with ESMTP id 75ABB10D804 for ; Wed, 28 Jun 2017 22:44:16 +0200 (CEST) Received: from bahia.lan (gar31-1-82-66-74-139.fbx.proxad.net [82.66.74.139]) (Authenticated sender: groug@kaod.org) by player799.ha.ovh.net (Postfix) with ESMTPA id 2CF9852007A; Wed, 28 Jun 2017 22:44:13 +0200 (CEST) From: Greg Kurz To: qemu-devel@nongnu.org Date: Wed, 28 Jun 2017 22:44:12 +0200 Message-ID: <149868265291.23385.12194127796890808447.stgit@bahia.lan> In-Reply-To: <149868263738.23385.16723444264552987199.stgit@bahia.lan> References: <149868263738.23385.16723444264552987199.stgit@bahia.lan> User-Agent: StGit/0.17.1-20-gc0b1b-dirty MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable X-Ovh-Tracer-Id: 1522216675609975217 X-VR-SPAMSTATE: OK X-VR-SPAMSCORE: -100 X-VR-SPAMCAUSE: gggruggvucftvghtrhhoucdtuddrfeelkedrtddvgdduheeiucetufdoteggodetrfdotffvucfrrhhofhhilhgvmecuqfggjfdpvefjgfevmfevgfenuceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddm X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 91.121.55.239 Subject: [Qemu-devel] [PATCH v5 1/5] virtio-9p: record element after sanity checks X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Stefano Stabellini , Greg Kurz , "Michael S. Tsirkin" Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail: RSF_0 Z_629925259 SPT_0 If the guest sends a malformed request, we end up with a dangling pointer in V9fsVirtioState. This doesn't seem to cause any bug, but let's remove this side effect anyway. Signed-off-by: Greg Kurz Reviewed-by: Michael S. Tsirkin --- hw/9pfs/virtio-9p-device.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c index 245abd8aaef1..3380bfc0c551 100644 --- a/hw/9pfs/virtio-9p-device.c +++ b/hw/9pfs/virtio-9p-device.c @@ -61,7 +61,6 @@ static void handle_9p_output(VirtIODevice *vdev, VirtQueu= e *vq) } QEMU_BUILD_BUG_ON(sizeof(out) !=3D 7); =20 - v->elems[pdu->idx] =3D elem; len =3D iov_to_buf(elem->out_sg, elem->out_num, 0, &out, sizeof(out)); if (len !=3D sizeof(out)) { @@ -70,6 +69,8 @@ static void handle_9p_output(VirtIODevice *vdev, VirtQueu= e *vq) goto out_free_req; } =20 + v->elems[pdu->idx] =3D elem; + pdu_submit(pdu, &out); } =20 From nobody Thu May 2 05:48:00 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) client-ip=208.118.235.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; Authentication-Results: mx.zoho.com; spf=pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org; Return-Path: Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) by mx.zohomail.com with SMTPS id 1498683009922148.54607080348137; Wed, 28 Jun 2017 13:50:09 -0700 (PDT) Received: from localhost ([::1]:35602 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dQJuZ-0003VP-HV for importer@patchew.org; Wed, 28 Jun 2017 16:50:07 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40189) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dQJp8-00071J-00 for qemu-devel@nongnu.org; Wed, 28 Jun 2017 16:44:33 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1dQJp4-00045A-UP for qemu-devel@nongnu.org; Wed, 28 Jun 2017 16:44:30 -0400 Received: from 9.mo5.mail-out.ovh.net ([178.32.96.204]:53156) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1dQJp4-00044j-NY for qemu-devel@nongnu.org; Wed, 28 Jun 2017 16:44:26 -0400 Received: from player799.ha.ovh.net (b9.ovh.net [213.186.33.59]) by mo5.mail-out.ovh.net (Postfix) with ESMTP id 527CB10DD64 for ; Wed, 28 Jun 2017 22:44:25 +0200 (CEST) Received: from bahia.lan (gar31-1-82-66-74-139.fbx.proxad.net [82.66.74.139]) (Authenticated sender: groug@kaod.org) by player799.ha.ovh.net (Postfix) with ESMTPA id BCAAC52007A; Wed, 28 Jun 2017 22:44:21 +0200 (CEST) From: Greg Kurz To: qemu-devel@nongnu.org Date: Wed, 28 Jun 2017 22:44:21 +0200 Message-ID: <149868266150.23385.2902323487462062636.stgit@bahia.lan> In-Reply-To: <149868263738.23385.16723444264552987199.stgit@bahia.lan> References: <149868263738.23385.16723444264552987199.stgit@bahia.lan> User-Agent: StGit/0.17.1-20-gc0b1b-dirty MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable X-Ovh-Tracer-Id: 1524749950012987825 X-VR-SPAMSTATE: OK X-VR-SPAMSCORE: -100 X-VR-SPAMCAUSE: gggruggvucftvghtrhhoucdtuddrfeelkedrtddvgdduheeiucetufdoteggodetrfdotffvucfrrhhofhhilhgvmecuqfggjfdpvefjgfevmfevgfenuceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddm X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 178.32.96.204 Subject: [Qemu-devel] [PATCH v5 2/5] virtio-9p: message header is 7-byte long X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Stefano Stabellini , Greg Kurz , "Michael S. Tsirkin" Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail: RSF_0 Z_629925259 SPT_0 The 9p spec at http://man.cat-v.org/plan_9/5/intro reads: "Each 9P message begins with a four-byte size field specify- ing the length in bytes of the complete message including the four bytes of the size field itself. The next byte is the message type, one of the constants in the enumeration in the include file . The next two bytes are an iden- tifying tag, described below." ie, each message starts with a 7-byte long header. The core 9P code already assumes this pretty much everywhere. This patch does the following: - makes the assumption explicit in the common 9p.h header, since it isn't related to the transport - open codes the header size in handle_9p_output() and hardens the sanity check on the space needed for the reply message Signed-off-by: Greg Kurz Acked-by: Stefano Stabellini Reviewed-by: Michael S. Tsirkin --- hw/9pfs/9p.h | 5 +++++ hw/9pfs/virtio-9p-device.c | 8 +++----- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/hw/9pfs/9p.h b/hw/9pfs/9p.h index c886ba78d2ee..aac1b0b2ce3d 100644 --- a/hw/9pfs/9p.h +++ b/hw/9pfs/9p.h @@ -124,6 +124,11 @@ typedef struct { uint8_t id; uint16_t tag_le; } QEMU_PACKED P9MsgHeader; +/* According to the specification, 9p messages start with a 7-byte header. + * Since most of the code uses this header size in literal form, we must be + * sure this is indeed the case. + */ +QEMU_BUILD_BUG_ON(sizeof(P9MsgHeader) !=3D 7); =20 struct V9fsPDU { diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c index 3380bfc0c551..1a68c1622d3a 100644 --- a/hw/9pfs/virtio-9p-device.c +++ b/hw/9pfs/virtio-9p-device.c @@ -53,17 +53,15 @@ static void handle_9p_output(VirtIODevice *vdev, VirtQu= eue *vq) goto out_free_pdu; } =20 - if (elem->in_num =3D=3D 0) { + if (iov_size(elem->in_sg, elem->in_num) < 7) { virtio_error(vdev, "The guest sent a VirtFS request without space fo= r " "the reply"); goto out_free_req; } - QEMU_BUILD_BUG_ON(sizeof(out) !=3D 7); =20 - len =3D iov_to_buf(elem->out_sg, elem->out_num, 0, - &out, sizeof(out)); - if (len !=3D sizeof(out)) { + len =3D iov_to_buf(elem->out_sg, elem->out_num, 0, &out, 7); + if (len !=3D 7) { virtio_error(vdev, "The guest sent a malformed VirtFS request:= " "header size is %zd, should be 7", len); goto out_free_req; From nobody Thu May 2 05:48:00 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) client-ip=208.118.235.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; Authentication-Results: mx.zoho.com; spf=pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org; Return-Path: Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) by mx.zohomail.com with SMTPS id 1498683129186788.9608990426051; Wed, 28 Jun 2017 13:52:09 -0700 (PDT) Received: from localhost ([::1]:35613 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dQJwV-00057e-Mk for importer@patchew.org; Wed, 28 Jun 2017 16:52:07 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40243) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dQJpG-000799-Ob for qemu-devel@nongnu.org; Wed, 28 Jun 2017 16:44:41 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1dQJpD-00048I-Li for qemu-devel@nongnu.org; Wed, 28 Jun 2017 16:44:38 -0400 Received: from 8.mo5.mail-out.ovh.net ([178.32.116.78]:49580) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1dQJpD-00047o-Ci for qemu-devel@nongnu.org; Wed, 28 Jun 2017 16:44:35 -0400 Received: from player799.ha.ovh.net (b9.ovh.net [213.186.33.59]) by mo5.mail-out.ovh.net (Postfix) with ESMTP id E67B110DDA2 for ; Wed, 28 Jun 2017 22:44:33 +0200 (CEST) Received: from bahia.lan (gar31-1-82-66-74-139.fbx.proxad.net [82.66.74.139]) (Authenticated sender: groug@kaod.org) by player799.ha.ovh.net (Postfix) with ESMTPA id 9ABE352007A; Wed, 28 Jun 2017 22:44:30 +0200 (CEST) From: Greg Kurz To: qemu-devel@nongnu.org Date: Wed, 28 Jun 2017 22:44:30 +0200 Message-ID: <149868267036.23385.17703911111121496563.stgit@bahia.lan> In-Reply-To: <149868263738.23385.16723444264552987199.stgit@bahia.lan> References: <149868263738.23385.16723444264552987199.stgit@bahia.lan> User-Agent: StGit/0.17.1-20-gc0b1b-dirty MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable X-Ovh-Tracer-Id: 1527001752625912241 X-VR-SPAMSTATE: OK X-VR-SPAMSCORE: -100 X-VR-SPAMCAUSE: gggruggvucftvghtrhhoucdtuddrfeelkedrtddvgdduheeiucetufdoteggodetrfdotffvucfrrhhofhhilhgvmecuqfggjfdpvefjgfevmfevgfenuceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddm X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 178.32.116.78 Subject: [Qemu-devel] [PATCH v5 3/5] virtio-9p: break device if buffers are misconfigured X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Stefano Stabellini , Greg Kurz , "Michael S. Tsirkin" Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail: RSF_0 Z_629925259 SPT_0 The 9P protocol is transport agnostic: if the guest misconfigured the buffers, the best we can do is to set the broken flag on the device. Since virtio_pdu_vmarshal() may be called by several active PDUs, we check if the transport isn't broken already to avoid printing extra error messages. Signed-off-by: Greg Kurz Reviewed-by: Michael S. Tsirkin --- v5: - use ssize_t variable in virtio_pdu_v[un]marshal() - drop remaining vdev->broken check (MST suggested to discuss calling virtio_error() when the device is already broken to a separate thread) --- hw/9pfs/9p.c | 2 +- hw/9pfs/9p.h | 2 +- hw/9pfs/virtio-9p-device.c | 40 ++++++++++++++++++++++++++++++++++++---- hw/9pfs/xen-9p-backend.c | 3 ++- 4 files changed, 40 insertions(+), 7 deletions(-) diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c index 96d268334865..da0d6da65b45 100644 --- a/hw/9pfs/9p.c +++ b/hw/9pfs/9p.c @@ -1664,7 +1664,7 @@ static void v9fs_init_qiov_from_pdu(QEMUIOVector *qio= v, V9fsPDU *pdu, unsigned int niov; =20 if (is_write) { - pdu->s->transport->init_out_iov_from_pdu(pdu, &iov, &niov); + pdu->s->transport->init_out_iov_from_pdu(pdu, &iov, &niov, size + = skip); } else { pdu->s->transport->init_in_iov_from_pdu(pdu, &iov, &niov, size + s= kip); } diff --git a/hw/9pfs/9p.h b/hw/9pfs/9p.h index aac1b0b2ce3d..d1cfeaf10e4f 100644 --- a/hw/9pfs/9p.h +++ b/hw/9pfs/9p.h @@ -363,7 +363,7 @@ struct V9fsTransport { void (*init_in_iov_from_pdu)(V9fsPDU *pdu, struct iovec **piov, unsigned int *pniov, size_t size); void (*init_out_iov_from_pdu)(V9fsPDU *pdu, struct iovec **piov, - unsigned int *pniov); + unsigned int *pniov, size_t size); void (*push_and_notify)(V9fsPDU *pdu); }; =20 diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c index 1a68c1622d3a..62650b0a6b99 100644 --- a/hw/9pfs/virtio-9p-device.c +++ b/hw/9pfs/virtio-9p-device.c @@ -146,8 +146,16 @@ static ssize_t virtio_pdu_vmarshal(V9fsPDU *pdu, size_= t offset, V9fsState *s =3D pdu->s; V9fsVirtioState *v =3D container_of(s, V9fsVirtioState, state); VirtQueueElement *elem =3D v->elems[pdu->idx]; + ssize_t ret; =20 - return v9fs_iov_vmarshal(elem->in_sg, elem->in_num, offset, 1, fmt, ap= ); + ret =3D v9fs_iov_vmarshal(elem->in_sg, elem->in_num, offset, 1, fmt, a= p); + if (ret < 0) { + VirtIODevice *vdev =3D VIRTIO_DEVICE(v); + + virtio_error(vdev, "Failed to encode VirtFS reply type %d", + pdu->id + 1); + } + return ret; } =20 static ssize_t virtio_pdu_vunmarshal(V9fsPDU *pdu, size_t offset, @@ -156,28 +164,52 @@ static ssize_t virtio_pdu_vunmarshal(V9fsPDU *pdu, si= ze_t offset, V9fsState *s =3D pdu->s; V9fsVirtioState *v =3D container_of(s, V9fsVirtioState, state); VirtQueueElement *elem =3D v->elems[pdu->idx]; + ssize_t ret; + + ret =3D v9fs_iov_vunmarshal(elem->out_sg, elem->out_num, offset, 1, fm= t, ap); + if (ret < 0) { + VirtIODevice *vdev =3D VIRTIO_DEVICE(v); =20 - return v9fs_iov_vunmarshal(elem->out_sg, elem->out_num, offset, 1, fmt= , ap); + virtio_error(vdev, "Failed to decode VirtFS request type %d", pdu-= >id); + } + return ret; } =20 -/* The size parameter is used by other transports. Do not drop it. */ static void virtio_init_in_iov_from_pdu(V9fsPDU *pdu, struct iovec **piov, unsigned int *pniov, size_t size) { V9fsState *s =3D pdu->s; V9fsVirtioState *v =3D container_of(s, V9fsVirtioState, state); VirtQueueElement *elem =3D v->elems[pdu->idx]; + size_t buf_size =3D iov_size(elem->in_sg, elem->in_num); + + if (buf_size < size) { + VirtIODevice *vdev =3D VIRTIO_DEVICE(v); + + virtio_error(vdev, + "VirtFS reply type %d needs %zu bytes, buffer has %zu= ", + pdu->id + 1, size, buf_size); + } =20 *piov =3D elem->in_sg; *pniov =3D elem->in_num; } =20 static void virtio_init_out_iov_from_pdu(V9fsPDU *pdu, struct iovec **piov, - unsigned int *pniov) + unsigned int *pniov, size_t size) { V9fsState *s =3D pdu->s; V9fsVirtioState *v =3D container_of(s, V9fsVirtioState, state); VirtQueueElement *elem =3D v->elems[pdu->idx]; + size_t buf_size =3D iov_size(elem->out_sg, elem->out_num); + + if (buf_size < size) { + VirtIODevice *vdev =3D VIRTIO_DEVICE(v); + + virtio_error(vdev, + "VirtFS request type %d needs %zu bytes, buffer has %= zu", + pdu->id, size, buf_size); + } =20 *piov =3D elem->out_sg; *pniov =3D elem->out_num; diff --git a/hw/9pfs/xen-9p-backend.c b/hw/9pfs/xen-9p-backend.c index 922cc967be63..a82cf817fe45 100644 --- a/hw/9pfs/xen-9p-backend.c +++ b/hw/9pfs/xen-9p-backend.c @@ -147,7 +147,8 @@ static ssize_t xen_9pfs_pdu_vunmarshal(V9fsPDU *pdu, =20 static void xen_9pfs_init_out_iov_from_pdu(V9fsPDU *pdu, struct iovec **piov, - unsigned int *pniov) + unsigned int *pniov, + size_t size) { Xen9pfsDev *xen_9pfs =3D container_of(pdu->s, Xen9pfsDev, state); Xen9pfsRing *ring =3D &xen_9pfs->rings[pdu->tag % xen_9pfs->num_rings]; From nobody Thu May 2 05:48:00 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) client-ip=208.118.235.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; Authentication-Results: mx.zoho.com; spf=pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org; Return-Path: Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) by mx.zohomail.com with SMTPS id 149868287294766.05320955598813; Wed, 28 Jun 2017 13:47:52 -0700 (PDT) Received: from localhost ([::1]:35595 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dQJsN-00014j-Hf for importer@patchew.org; Wed, 28 Jun 2017 16:47:51 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40275) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dQJpP-0007Ho-Et for qemu-devel@nongnu.org; Wed, 28 Jun 2017 16:44:48 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1dQJpM-0004B5-Aj for qemu-devel@nongnu.org; Wed, 28 Jun 2017 16:44:47 -0400 Received: from 5.mo5.mail-out.ovh.net ([87.98.173.103]:56961) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1dQJpM-0004AK-0L for qemu-devel@nongnu.org; Wed, 28 Jun 2017 16:44:44 -0400 Received: from player799.ha.ovh.net (b9.ovh.net [213.186.33.59]) by mo5.mail-out.ovh.net (Postfix) with ESMTP id 89E7910DD66 for ; Wed, 28 Jun 2017 22:44:42 +0200 (CEST) Received: from bahia.lan (gar31-1-82-66-74-139.fbx.proxad.net [82.66.74.139]) (Authenticated sender: groug@kaod.org) by player799.ha.ovh.net (Postfix) with ESMTPA id 3C9C252007A; Wed, 28 Jun 2017 22:44:39 +0200 (CEST) From: Greg Kurz To: qemu-devel@nongnu.org Date: Wed, 28 Jun 2017 22:44:39 +0200 Message-ID: <149868267896.23385.6937506856085271797.stgit@bahia.lan> In-Reply-To: <149868263738.23385.16723444264552987199.stgit@bahia.lan> References: <149868263738.23385.16723444264552987199.stgit@bahia.lan> User-Agent: StGit/0.17.1-20-gc0b1b-dirty MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable X-Ovh-Tracer-Id: 1529535026195241393 X-VR-SPAMSTATE: OK X-VR-SPAMSCORE: -100 X-VR-SPAMCAUSE: gggruggvucftvghtrhhoucdtuddrfeelkedrtddvgdduheeiucetufdoteggodetrfdotffvucfrrhhofhhilhgvmecuqfggjfdpvefjgfevmfevgfenuceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddm X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 87.98.173.103 Subject: [Qemu-devel] [PATCH v5 4/5] xen-9pfs: disconnect if buffers are misconfigured X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Stefano Stabellini , Greg Kurz , "Michael S. Tsirkin" Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail: RSF_0 Z_629925259 SPT_0 From: Stefano Stabellini Implement xen_9pfs_disconnect by unbinding the event channels. On xen_9pfs_free, call disconnect if any event channels haven't been disconnected. If the frontend misconfigured the buffers set the backend to "Closing" and disconnect it. Misconfigurations include requesting a read of more bytes than available on the ring buffer, or claiming to be writing more data than available on the ring buffer. Signed-off-by: Stefano Stabellini Signed-off-by: Greg Kurz --- Changes: - fixed some whitespace damages - xen_9pfs_receive() now returns 0 in case of short read --- hw/9pfs/xen-9p-backend.c | 85 +++++++++++++++++++++++++++++++++++-------= ---- 1 file changed, 64 insertions(+), 21 deletions(-) diff --git a/hw/9pfs/xen-9p-backend.c b/hw/9pfs/xen-9p-backend.c index a82cf817fe45..ee87f08926a2 100644 --- a/hw/9pfs/xen-9p-backend.c +++ b/hw/9pfs/xen-9p-backend.c @@ -54,6 +54,8 @@ typedef struct Xen9pfsDev { Xen9pfsRing *rings; } Xen9pfsDev; =20 +static void xen_9pfs_disconnect(struct XenDevice *xendev); + static void xen_9pfs_in_sg(Xen9pfsRing *ring, struct iovec *in_sg, int *num, @@ -125,10 +127,19 @@ static ssize_t xen_9pfs_pdu_vmarshal(V9fsPDU *pdu, Xen9pfsDev *xen_9pfs =3D container_of(pdu->s, Xen9pfsDev, state); struct iovec in_sg[2]; int num; + ssize_t ret; =20 xen_9pfs_in_sg(&xen_9pfs->rings[pdu->tag % xen_9pfs->num_rings], in_sg, &num, pdu->idx, ROUND_UP(offset + 128, 512)); - return v9fs_iov_vmarshal(in_sg, num, offset, 0, fmt, ap); + + ret =3D v9fs_iov_vmarshal(in_sg, num, offset, 0, fmt, ap); + if (ret < 0) { + xen_pv_printf(&xen_9pfs->xendev, 0, + "Failed to encode VirtFS request type %d\n", pdu->id= + 1); + xen_be_set_state(&xen_9pfs->xendev, XenbusStateClosing); + xen_9pfs_disconnect(&xen_9pfs->xendev); + } + return ret; } =20 static ssize_t xen_9pfs_pdu_vunmarshal(V9fsPDU *pdu, @@ -139,10 +150,19 @@ static ssize_t xen_9pfs_pdu_vunmarshal(V9fsPDU *pdu, Xen9pfsDev *xen_9pfs =3D container_of(pdu->s, Xen9pfsDev, state); struct iovec out_sg[2]; int num; + ssize_t ret; =20 xen_9pfs_out_sg(&xen_9pfs->rings[pdu->tag % xen_9pfs->num_rings], out_sg, &num, pdu->idx); - return v9fs_iov_vunmarshal(out_sg, num, offset, 0, fmt, ap); + + ret =3D v9fs_iov_vunmarshal(out_sg, num, offset, 0, fmt, ap); + if (ret < 0) { + xen_pv_printf(&xen_9pfs->xendev, 0, + "Failed to decode VirtFS request type %d\n", pdu->id= ); + xen_be_set_state(&xen_9pfs->xendev, XenbusStateClosing); + xen_9pfs_disconnect(&xen_9pfs->xendev); + } + return ret; } =20 static void xen_9pfs_init_out_iov_from_pdu(V9fsPDU *pdu, @@ -170,11 +190,22 @@ static void xen_9pfs_init_in_iov_from_pdu(V9fsPDU *pd= u, Xen9pfsDev *xen_9pfs =3D container_of(pdu->s, Xen9pfsDev, state); Xen9pfsRing *ring =3D &xen_9pfs->rings[pdu->tag % xen_9pfs->num_rings]; int num; + size_t buf_size; =20 g_free(ring->sg); =20 ring->sg =3D g_malloc0(sizeof(*ring->sg) * 2); xen_9pfs_in_sg(ring, ring->sg, &num, pdu->idx, size); + + buf_size =3D iov_size(ring->sg, num); + if (buf_size < size) { + xen_pv_printf(&xen_9pfs->xendev, 0, "Xen 9pfs request type %d" + "needs %zu bytes, buffer has %zu\n", pdu->id, size, + buf_size); + xen_be_set_state(&xen_9pfs->xendev, XenbusStateClosing); + xen_9pfs_disconnect(&xen_9pfs->xendev); + } + *piov =3D ring->sg; *pniov =3D num; } @@ -218,7 +249,7 @@ static int xen_9pfs_init(struct XenDevice *xendev) static int xen_9pfs_receive(Xen9pfsRing *ring) { P9MsgHeader h; - RING_IDX cons, prod, masked_prod, masked_cons; + RING_IDX cons, prod, masked_prod, masked_cons, queued; V9fsPDU *pdu; =20 if (ring->inprogress) { @@ -229,8 +260,8 @@ static int xen_9pfs_receive(Xen9pfsRing *ring) prod =3D ring->intf->out_prod; xen_rmb(); =20 - if (xen_9pfs_queued(prod, cons, XEN_FLEX_RING_SIZE(ring->ring_order)) < - sizeof(h)) { + queued =3D xen_9pfs_queued(prod, cons, XEN_FLEX_RING_SIZE(ring->ring_o= rder)); + if (queued < sizeof(h)) { return 0; } ring->inprogress =3D true; @@ -241,6 +272,9 @@ static int xen_9pfs_receive(Xen9pfsRing *ring) xen_9pfs_read_packet((uint8_t *) &h, ring->ring.out, sizeof(h), masked_prod, &masked_cons, XEN_FLEX_RING_SIZE(ring->ring_order)); + if (queued < le32_to_cpu(h.size_le)) { + return 0; + } =20 /* cannot fail, because we only handle one request per ring at a time = */ pdu =3D pdu_alloc(&ring->priv->state); @@ -269,15 +303,30 @@ static void xen_9pfs_evtchn_event(void *opaque) qemu_bh_schedule(ring->bh); } =20 -static int xen_9pfs_free(struct XenDevice *xendev) +static void xen_9pfs_disconnect(struct XenDevice *xendev) { + Xen9pfsDev *xen_9pdev =3D container_of(xendev, Xen9pfsDev, xendev); int i; + + for (i =3D 0; i < xen_9pdev->num_rings; i++) { + if (xen_9pdev->rings[i].evtchndev !=3D NULL) { + qemu_set_fd_handler(xenevtchn_fd(xen_9pdev->rings[i].evtchndev= ), + NULL, NULL, NULL); + xenevtchn_unbind(xen_9pdev->rings[i].evtchndev, + xen_9pdev->rings[i].local_port); + xen_9pdev->rings[i].evtchndev =3D NULL; + } + } +} + +static int xen_9pfs_free(struct XenDevice *xendev) +{ Xen9pfsDev *xen_9pdev =3D container_of(xendev, Xen9pfsDev, xendev); + int i; =20 - g_free(xen_9pdev->id); - g_free(xen_9pdev->tag); - g_free(xen_9pdev->path); - g_free(xen_9pdev->security_model); + if (xen_9pdev->rings[0].evtchndev !=3D NULL) { + xen_9pfs_disconnect(xendev); + } =20 for (i =3D 0; i < xen_9pdev->num_rings; i++) { if (xen_9pdev->rings[i].data !=3D NULL) { @@ -290,16 +339,15 @@ static int xen_9pfs_free(struct XenDevice *xendev) xen_9pdev->rings[i].intf, 1); } - if (xen_9pdev->rings[i].evtchndev > 0) { - qemu_set_fd_handler(xenevtchn_fd(xen_9pdev->rings[i].evtchndev= ), - NULL, NULL, NULL); - xenevtchn_unbind(xen_9pdev->rings[i].evtchndev, - xen_9pdev->rings[i].local_port); - } if (xen_9pdev->rings[i].bh !=3D NULL) { qemu_bh_delete(xen_9pdev->rings[i].bh); } } + + g_free(xen_9pdev->id); + g_free(xen_9pdev->tag); + g_free(xen_9pdev->path); + g_free(xen_9pdev->security_model); g_free(xen_9pdev->rings); return 0; } @@ -423,11 +471,6 @@ static void xen_9pfs_alloc(struct XenDevice *xendev) xenstore_write_be_int(xendev, "max-ring-page-order", MAX_RING_ORDER); } =20 -static void xen_9pfs_disconnect(struct XenDevice *xendev) -{ - /* Dynamic hotplug of PV filesystems at runtime is not supported. */ -} - struct XenDevOps xen_9pfs_ops =3D { .size =3D sizeof(Xen9pfsDev), .flags =3D DEVOPS_FLAG_NEED_GNTDEV, From nobody Thu May 2 05:48:00 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) client-ip=208.118.235.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; Authentication-Results: mx.zoho.com; spf=pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org; Return-Path: Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) by mx.zohomail.com with SMTPS id 1498683216288944.9548117055142; Wed, 28 Jun 2017 13:53:36 -0700 (PDT) Received: from localhost ([::1]:35618 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dQJxr-0006DA-Qd for importer@patchew.org; Wed, 28 Jun 2017 16:53:31 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40329) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dQJpX-0007Mc-Pk for qemu-devel@nongnu.org; Wed, 28 Jun 2017 16:44:58 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1dQJpU-0004Fg-Ol for qemu-devel@nongnu.org; Wed, 28 Jun 2017 16:44:55 -0400 Received: from 7.mo5.mail-out.ovh.net ([178.32.124.100]:59129) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1dQJpU-0004F5-Gv for qemu-devel@nongnu.org; Wed, 28 Jun 2017 16:44:52 -0400 Received: from player799.ha.ovh.net (b9.ovh.net [213.186.33.59]) by mo5.mail-out.ovh.net (Postfix) with ESMTP id 236C910DD66 for ; Wed, 28 Jun 2017 22:44:51 +0200 (CEST) Received: from bahia.lan (gar31-1-82-66-74-139.fbx.proxad.net [82.66.74.139]) (Authenticated sender: groug@kaod.org) by player799.ha.ovh.net (Postfix) with ESMTPA id D28DA52007B; Wed, 28 Jun 2017 22:44:47 +0200 (CEST) From: Greg Kurz To: qemu-devel@nongnu.org Date: Wed, 28 Jun 2017 22:44:47 +0200 Message-ID: <149868268758.23385.1547307280569014820.stgit@bahia.lan> In-Reply-To: <149868263738.23385.16723444264552987199.stgit@bahia.lan> References: <149868263738.23385.16723444264552987199.stgit@bahia.lan> User-Agent: StGit/0.17.1-20-gc0b1b-dirty MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable X-Ovh-Tracer-Id: 1532068300331653553 X-VR-SPAMSTATE: OK X-VR-SPAMSCORE: -100 X-VR-SPAMCAUSE: gggruggvucftvghtrhhoucdtuddrfeelkedrtddvgdduheeiucetufdoteggodetrfdotffvucfrrhhofhhilhgvmecuqfggjfdpvefjgfevmfevgfenuceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddm X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 178.32.124.100 Subject: [Qemu-devel] [PATCH v5 5/5] 9pfs: handle transport errors in pdu_complete() X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Stefano Stabellini , Greg Kurz , "Michael S. Tsirkin" Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail: RSF_0 Z_629925259 SPT_0 Contrary to what is written in the comment, a buggy guest can misconfigure the transport buffers and pdu_marshal() may return an error. If this ever happens, it is up to the transport layer to handle the situation (9P is transport agnostic). This fixes Coverity issue CID1348518. Signed-off-by: Greg Kurz Reviewed-by: Stefano Stabellini --- hw/9pfs/9p.c | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c index da0d6da65b45..f7871e6f3e5a 100644 --- a/hw/9pfs/9p.c +++ b/hw/9pfs/9p.c @@ -624,15 +624,11 @@ void pdu_free(V9fsPDU *pdu) QLIST_INSERT_HEAD(&s->free_list, pdu, next); } =20 -/* - * We don't do error checking for pdu_marshal/unmarshal here - * because we always expect to have enough space to encode - * error details - */ static void coroutine_fn pdu_complete(V9fsPDU *pdu, ssize_t len) { int8_t id =3D pdu->id + 1; /* Response */ V9fsState *s =3D pdu->s; + int ret; =20 if (len < 0) { int err =3D -len; @@ -644,11 +640,19 @@ static void coroutine_fn pdu_complete(V9fsPDU *pdu, s= size_t len) str.data =3D strerror(err); str.size =3D strlen(str.data); =20 - len +=3D pdu_marshal(pdu, len, "s", &str); + ret =3D pdu_marshal(pdu, len, "s", &str); + if (ret < 0) { + goto out_notify; + } + len +=3D ret; id =3D P9_RERROR; } =20 - len +=3D pdu_marshal(pdu, len, "d", err); + ret =3D pdu_marshal(pdu, len, "d", err); + if (ret < 0) { + goto out_notify; + } + len +=3D ret; =20 if (s->proto_version =3D=3D V9FS_PROTO_2000L) { id =3D P9_RLERROR; @@ -657,12 +661,15 @@ static void coroutine_fn pdu_complete(V9fsPDU *pdu, s= size_t len) } =20 /* fill out the header */ - pdu_marshal(pdu, 0, "dbw", (int32_t)len, id, pdu->tag); + if (pdu_marshal(pdu, 0, "dbw", (int32_t)len, id, pdu->tag) < 0) { + goto out_notify; + } =20 /* keep these in sync */ pdu->size =3D len; pdu->id =3D id; =20 +out_notify: pdu->s->transport->push_and_notify(pdu); =20 /* Now wakeup anybody waiting in flush for this request */