From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45952) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VDoUF-0007lV-V8 for qemu-devel@nongnu.org; Mon, 26 Aug 2013 00:33:15 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VDoU3-0005LO-3H for qemu-devel@nongnu.org; Mon, 26 Aug 2013 00:33:07 -0400 Received: from e28smtp02.in.ibm.com ([122.248.162.2]:51278) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VDoU2-0005KS-FQ for qemu-devel@nongnu.org; Mon, 26 Aug 2013 00:32:55 -0400 Received: from /spool/local by e28smtp02.in.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Mon, 26 Aug 2013 09:52:31 +0530 From: Nikunj A Dadhania In-Reply-To: <1377468637.3819.27.camel@pasglop> References: <1377249737-12570-1-git-send-email-aik@ozlabs.ru> <24C2B209-2082-4AF8-A8FB-1FF8A8B7751B@suse.de> <1377468637.3819.27.camel@pasglop> Date: Mon, 26 Aug 2013 10:02:44 +0530 Message-ID: <87vc2tysur.fsf@linux.vnet.ibm.com> MIME-Version: 1.0 Content-Type: text/plain Subject: Re: [Qemu-devel] [PATCH] spapr-vscsi: Adding VSCSI capabilities List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Benjamin Herrenschmidt , Alexander Graf Cc: Alexey Kardashevskiy , Paolo Bonzini , qemu-ppc@nongnu.org, qemu-devel@nongnu.org Benjamin Herrenschmidt writes: > On Sun, 2013-08-25 at 17:41 +0100, Alexander Graf wrote: >> > + vcap = &req->iu.mad.capabilities; >> > + rc = spapr_vio_dma_read(&s->vdev, be64_to_cpu(vcap->buffer), >> > + &cap, >> be16_to_cpu(vcap->common.length)); >> >> While I don't think any harm could happen from it, this could lead to >> a potential timing attack where we read and write from different >> locations in memory if the guest swizzles the request while we're >> processing it. > > BTW. While I disagree with your initial comment ... is there any bound > checking here ? That looks like potential stack corruption unless I > miss something if the guest passes a too big length... > > So at least the length should be read once, bound-checked, then the read > done with the result (don't bound check and read again, that would be > indeed racy). How about the below patch: From: Nikunj A Dadhania This implements capabilities exchange between host and client. As at the moment no capability is supported, put zero flags everywhere and return. Signed-off-by: Nikunj A Dadhania --- hw/scsi/spapr_vscsi.c | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/hw/scsi/spapr_vscsi.c b/hw/scsi/spapr_vscsi.c index e9090e5..fae3644 100644 --- a/hw/scsi/spapr_vscsi.c +++ b/hw/scsi/spapr_vscsi.c @@ -858,6 +858,40 @@ static int vscsi_send_adapter_info(VSCSIState *s, vscsi_req *req) return vscsi_send_iu(s, req, sizeof(*sinfo), VIOSRP_MAD_FORMAT); } +static int vscsi_send_capabilities(VSCSIState *s, vscsi_req *req) +{ + struct viosrp_capabilities *vcap; + struct capabilities cap; + uint16_t len = 0; + int rc = true; + + vcap = &req->iu.mad.capabilities; + len = be16_to_cpu(vcap->common.length); + if (len > sizeof(&cap)) { + fprintf(stderr, "vscsi_send_capabilities: size out of bound !\n"); + goto error_out; + } + rc = spapr_vio_dma_read(&s->vdev, be64_to_cpu(vcap->buffer), + &cap, len); + if (rc) { + fprintf(stderr, "vscsi_send_capabilities: DMA read failure !\n"); + } + + cap.flags = 0; + cap.migration.ecl = 0; + cap.reserve.type = 0; + cap.migration.common.server_support = 0; + cap.reserve.common.server_support = 0; + rc = spapr_vio_dma_write(&s->vdev, be64_to_cpu(vcap->buffer), + &cap, len); + if (rc) { + fprintf(stderr, "vscsi_send_capabilities: DMA write failure !\n"); + } +error_out: + vcap->common.status = rc ? cpu_to_be32(1) : 0; + return vscsi_send_iu(s, req, sizeof(*vcap), VIOSRP_MAD_FORMAT); +} + static int vscsi_handle_mad_req(VSCSIState *s, vscsi_req *req) { union mad_iu *mad = &req->iu.mad; @@ -878,6 +912,9 @@ static int vscsi_handle_mad_req(VSCSIState *s, vscsi_req *req) mad->host_config.common.status = cpu_to_be16(1); vscsi_send_iu(s, req, sizeof(mad->host_config), VIOSRP_MAD_FORMAT); break; + case VIOSRP_CAPABILITIES_TYPE: + vscsi_send_capabilities(s, req); + break; default: fprintf(stderr, "VSCSI: Unknown MAD type %02x\n", be32_to_cpu(mad->empty_iu.common.type)); -- 1.8.3.1