[Qemu-devel] [PATCH] iSCSI fix crash when using virtio and libiscsi

[Qemu-devel] [PATCH] iSCSI fix crash when using virtio and libiscsi

[Ronnie Sahlberg]

Stefan, List

Please find a patch that fixes the crashes for using virtio with libiscsi.
The problem was that block/iscsi.c always assumed we got a plain buffer to read data into, and when we got an iovector array instead we would overwrite pointers with garbage and crash.

Since we can get iovectors for the write case as well I have added a fix for when the guest is writing data to the target to handle the iovector case as well.


The new calls added are not protected with (LIBISCSI_FEATURE_IOVECTOR) checks
since anyone building a new/current version of qemu should probably also build
against a current libiscsi.
I will send patches later to remove the current (LIBISCSI_FEATURE_IOVECTOR) checks in the rest of the file.


regards
ronnie sahlberg

[Qemu-devel] [PATCH] Fix iSCSI crash on SG_IO with an iovector

[Ronnie Sahlberg]

Don't assume that SG_IO is always invoked with a simple buffer,
check the iovec_count and if it is > 1 then we need to pass an array
of iovectors to libiscsi instead of just a plain buffer.

Signed-off-by: Ronnie Sahlberg <ronniesahlberg@gmail.com>
---
 block/iscsi.c |   31 ++++++++++++++++++++++++-------
 1 files changed, 24 insertions(+), 7 deletions(-)

diff --git a/block/iscsi.c b/block/iscsi.c
index 0bbf0b1..2d1cb4e 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -727,25 +727,42 @@ static BlockDriverAIOCB *iscsi_aio_ioctl(BlockDriverState *bs,
     memcpy(&acb->task->cdb[0], acb->ioh->cmdp, acb->ioh->cmd_len);
     acb->task->expxferlen = acb->ioh->dxfer_len;
 
+    data.size = 0;
     if (acb->task->xfer_dir == SCSI_XFER_WRITE) {
-        data.data = acb->ioh->dxferp;
-        data.size = acb->ioh->dxfer_len;
+        if (acb->ioh->iovec_count == 0) {
+            data.data = acb->ioh->dxferp;
+            data.size = acb->ioh->dxfer_len;
+        }
     }
     if (iscsi_scsi_command_async(iscsi, iscsilun->lun, acb->task,
                                  iscsi_aio_ioctl_cb,
-                                 (acb->task->xfer_dir == SCSI_XFER_WRITE) ?
-                                     &data : NULL,
+                                 (data.size > 0) ? &data : NULL,
                                  acb) != 0) {
         scsi_free_scsi_task(acb->task);
         qemu_aio_release(acb);
         return NULL;
     }
 
+    /* We got an iovector for writing to the target */
+    if (acb->task->xfer_dir == SCSI_XFER_WRITE) {
+        if (acb->ioh->iovec_count > 0) {
+            scsi_task_set_iov_out(acb->task,
+                                  (struct scsi_iovec *) acb->ioh->dxferp,
+                                  acb->ioh->iovec_count);
+        }
+    }
+
     /* tell libiscsi to read straight into the buffer we got from ioctl */
     if (acb->task->xfer_dir == SCSI_XFER_READ) {
-        scsi_task_add_data_in_buffer(acb->task,
-                                     acb->ioh->dxfer_len,
-                                     acb->ioh->dxferp);
+        if (acb->ioh->iovec_count == 0) {
+            scsi_task_add_data_in_buffer(acb->task,
+                                         acb->ioh->dxfer_len,
+                                         acb->ioh->dxferp);
+        } else {
+            scsi_task_set_iov_in(acb->task,
+                                 (struct scsi_iovec *) acb->ioh->dxferp,
+                                 acb->ioh->iovec_count);
+        }
     }
 
     iscsi_set_events(iscsilun);
-- 
1.7.3.1

Re: [Qemu-devel] [PATCH] iSCSI fix crash when using virtio and libiscsi

[Paolo Bonzini]

Il 21/06/2013 04:32, Ronnie Sahlberg ha scritto:
> Stefan, List
> 
> Please find a patch that fixes the crashes for using virtio with libiscsi.
> The problem was that block/iscsi.c always assumed we got a plain buffer to read data into, and when we got an iovector array instead we would overwrite pointers with garbage and crash.
> 
> Since we can get iovectors for the write case as well I have added a fix for when the guest is writing data to the target to handle the iovector case as well.
> 
> 
> The new calls added are not protected with (LIBISCSI_FEATURE_IOVECTOR) checks
> since anyone building a new/current version of qemu should probably also build
> against a current libiscsi.

Not necessarily, you may build against an older libiscsi from the distro.

Can you resubmit with the checks intact?

Paolo

> I will send patches later to remove the current (LIBISCSI_FEATURE_IOVECTOR) checks in the rest of the file.

Re: [Qemu-devel] [PATCH] iSCSI fix crash when using virtio and libiscsi

[ronnie sahlberg]

I can add the checks and resubmit.

On Fri, Jun 21, 2013 at 12:38 AM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Il 21/06/2013 04:32, Ronnie Sahlberg ha scritto:
>> Stefan, List
>>
>> Please find a patch that fixes the crashes for using virtio with libiscsi.
>> The problem was that block/iscsi.c always assumed we got a plain buffer to read data into, and when we got an iovector array instead we would overwrite pointers with garbage and crash.
>>
>> Since we can get iovectors for the write case as well I have added a fix for when the guest is writing data to the target to handle the iovector case as well.
>>
>>
>> The new calls added are not protected with (LIBISCSI_FEATURE_IOVECTOR) checks
>> since anyone building a new/current version of qemu should probably also build
>> against a current libiscsi.
>
> Not necessarily, you may build against an older libiscsi from the distro.
>
> Can you resubmit with the checks intact?
>
> Paolo
>
>> I will send patches later to remove the current (LIBISCSI_FEATURE_IOVECTOR) checks in the rest of the file.
>