Re: [Qemu-devel] [PATCH] spapr-vscsi: Adding VSCSI capabilities

[Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>]

Alexander Graf <agraf@suse.de> writes:
> Am 26.08.2013 um 05:32 schrieb Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>:
>
>> Benjamin Herrenschmidt <benh@kernel.crashing.org> 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).
>> 

From: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>

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 <nikunj@linux.vnet.ibm.com>
---
 hw/scsi/spapr_vscsi.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 44 insertions(+)

diff --git a/hw/scsi/spapr_vscsi.c b/hw/scsi/spapr_vscsi.c
index e9090e5..0758263 100644
--- a/hw/scsi/spapr_vscsi.c
+++ b/hw/scsi/spapr_vscsi.c
@@ -858,6 +858,47 @@ 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;
+    uint64_t buffer;
+    int rc;
+
+    vcap = &req->iu.mad.capabilities;
+    len = be16_to_cpu(vcap->common.length);
+    buffer = be64_to_cpu(vcap->buffer);
+    if (len > sizeof(cap)) {
+        fprintf(stderr, "vscsi_send_capabilities: size out of bound !\n");
+        rc = H_PARAMETER;
+        goto error_out;
+    }
+    rc = spapr_vio_dma_read(&s->vdev, buffer, &cap, len);
+    if (rc)  {
+        fprintf(stderr, "vscsi_send_capabilities: DMA read failure !\n");
+    }
+
+    /*
+     * Current implementation does not suppport any migration or
+     * reservation capabilities. Construct the response telling the
+     * guest not to use them.
+     */
+    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, 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 +919,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