scsi: Dequeue requests before invoking completion callback

Submitted by Jan Kiszka on July 13, 2010, 12:13 p.m.

Details

Message ID 4C3C5879.4070609@siemens.com
State New
Headers show

Commit Message

Jan Kiszka July 13, 2010, 12:13 p.m.
The request completion callback of the LSI controller may start the next
request that can use the same tag as the completed one. As the latter is
still enqueued at that point, scsi_send_command will complain about the
tag reuse and cancel the completed request. That will cause a double
free later on when the completion path cleans up as well.

Fix this by dequeuing the request before invoking the callback.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---

This should fix bug 595438.

 hw/scsi-bus.c |   12 +++++++++++-
 hw/scsi.h     |    1 +
 2 files changed, 12 insertions(+), 1 deletions(-)

Comments

Aurelien Jarno July 22, 2010, 12:16 p.m.
On Tue, Jul 13, 2010 at 02:13:45PM +0200, Jan Kiszka wrote:
> The request completion callback of the LSI controller may start the next
> request that can use the same tag as the completed one. As the latter is
> still enqueued at that point, scsi_send_command will complain about the
> tag reuse and cancel the completed request. That will cause a double
> free later on when the completion path cleans up as well.
> 
> Fix this by dequeuing the request before invoking the callback.
> 
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
> 
> This should fix bug 595438.
> 
>  hw/scsi-bus.c |   12 +++++++++++-
>  hw/scsi.h     |    1 +
>  2 files changed, 12 insertions(+), 1 deletions(-)

Thanks, applied.

> diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
> index d69c74c..b860a09 100644
> --- a/hw/scsi-bus.c
> +++ b/hw/scsi-bus.c
> @@ -142,6 +142,7 @@ SCSIRequest *scsi_req_alloc(size_t size, SCSIDevice *d, uint32_t tag, uint32_t l
>      req->tag = tag;
>      req->lun = lun;
>      req->status = -1;
> +    req->enqueued = true;
>      QTAILQ_INSERT_TAIL(&d->requests, req, next);
>      return req;
>  }
> @@ -158,9 +159,17 @@ SCSIRequest *scsi_req_find(SCSIDevice *d, uint32_t tag)
>      return NULL;
>  }
>  
> +static void scsi_req_dequeue(SCSIRequest *req)
> +{
> +    if (req->enqueued) {
> +        QTAILQ_REMOVE(&req->dev->requests, req, next);
> +        req->enqueued = false;
> +    }
> +}
> +
>  void scsi_req_free(SCSIRequest *req)
>  {
> -    QTAILQ_REMOVE(&req->dev->requests, req, next);
> +    scsi_req_dequeue(req);
>      qemu_free(req);
>  }
>  
> @@ -512,6 +521,7 @@ void scsi_req_print(SCSIRequest *req)
>  void scsi_req_complete(SCSIRequest *req)
>  {
>      assert(req->status != -1);
> +    scsi_req_dequeue(req);
>      req->bus->complete(req->bus, SCSI_REASON_DONE,
>                         req->tag,
>                         req->status);
> diff --git a/hw/scsi.h b/hw/scsi.h
> index 4fbf1d5..cb06d6d 100644
> --- a/hw/scsi.h
> +++ b/hw/scsi.h
> @@ -43,6 +43,7 @@ typedef struct SCSIRequest {
>          enum SCSIXferMode mode;
>      } cmd;
>      BlockDriverAIOCB  *aiocb;
> +    bool enqueued;
>      QTAILQ_ENTRY(SCSIRequest) next;
>  } SCSIRequest;
>  
> -- 
> 1.7.1
> 
>

Patch hide | download patch | download mbox

diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
index d69c74c..b860a09 100644
--- a/hw/scsi-bus.c
+++ b/hw/scsi-bus.c
@@ -142,6 +142,7 @@  SCSIRequest *scsi_req_alloc(size_t size, SCSIDevice *d, uint32_t tag, uint32_t l
     req->tag = tag;
     req->lun = lun;
     req->status = -1;
+    req->enqueued = true;
     QTAILQ_INSERT_TAIL(&d->requests, req, next);
     return req;
 }
@@ -158,9 +159,17 @@  SCSIRequest *scsi_req_find(SCSIDevice *d, uint32_t tag)
     return NULL;
 }
 
+static void scsi_req_dequeue(SCSIRequest *req)
+{
+    if (req->enqueued) {
+        QTAILQ_REMOVE(&req->dev->requests, req, next);
+        req->enqueued = false;
+    }
+}
+
 void scsi_req_free(SCSIRequest *req)
 {
-    QTAILQ_REMOVE(&req->dev->requests, req, next);
+    scsi_req_dequeue(req);
     qemu_free(req);
 }
 
@@ -512,6 +521,7 @@  void scsi_req_print(SCSIRequest *req)
 void scsi_req_complete(SCSIRequest *req)
 {
     assert(req->status != -1);
+    scsi_req_dequeue(req);
     req->bus->complete(req->bus, SCSI_REASON_DONE,
                        req->tag,
                        req->status);
diff --git a/hw/scsi.h b/hw/scsi.h
index 4fbf1d5..cb06d6d 100644
--- a/hw/scsi.h
+++ b/hw/scsi.h
@@ -43,6 +43,7 @@  typedef struct SCSIRequest {
         enum SCSIXferMode mode;
     } cmd;
     BlockDriverAIOCB  *aiocb;
+    bool enqueued;
     QTAILQ_ENTRY(SCSIRequest) next;
 } SCSIRequest;