firewire: Add ref-counting for sbp2 orbs (fix command abortion)

This handles the case where we get the status write before getting the
complete_transaction callback ("status write for unknown orb").  In
this case, we just assume that the initial orb pointer transaction
succeeded and finish the orb.  To prevent the transaction callback
from touching freed memory, we ref-count the orb structures.

Signed-off-by: Kristian Høgsberg <krh@redhat.com>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>

1 files changed, 40 insertions, 9 deletions

diff --git a/drivers/firewire/fw-sbp2.c b/drivers/firewire/fw-sbp2.c
index ba816ef6def1..238730f75db1 100644
--- a/drivers/firewire/fw-sbp2.c
+++ b/drivers/firewire/fw-sbp2.c
@@ -159,6 +159,7 @@ struct sbp2_pointer {
 
 struct sbp2_orb {
         struct fw_transaction t;
+        struct kref kref;
         dma_addr_t request_bus;
         int rcode;
         struct sbp2_pointer pointer;
@@ -280,6 +281,14 @@ static const struct {
 };
 
 static void
+free_orb(struct kref *kref)
+{
+        struct sbp2_orb *orb = container_of(kref, struct sbp2_orb, kref);
+
+        kfree(orb);
+}
+
+static void
 sbp2_status_write(struct fw_card *card, struct fw_request *request,
                   int tcode, int destination, int source,
                   int generation, int speed,
@@ -312,8 +321,8 @@ sbp2_status_write(struct fw_card *card, struct fw_request *request,
         spin_lock_irqsave(&card->lock, flags);
         list_for_each_entry(orb, &sd->orb_list, link) {
                 if (STATUS_GET_ORB_HIGH(status) == 0 &&
-                    STATUS_GET_ORB_LOW(status) == orb->request_bus &&
-                    orb->rcode == RCODE_COMPLETE) {
+                    STATUS_GET_ORB_LOW(status) == orb->request_bus) {
+                        orb->rcode = RCODE_COMPLETE;
                         list_del(&orb->link);
                         break;
                 }
@@ -325,6 +334,8 @@ sbp2_status_write(struct fw_card *card, struct fw_request *request,
         else
                 fw_error("status write for unknown orb\n");
 
+        kref_put(&orb->kref, free_orb);
+
         fw_send_response(card, request, RCODE_COMPLETE);
 }
 
@@ -335,13 +346,27 @@ complete_transaction(struct fw_card *card, int rcode,
         struct sbp2_orb *orb = data;
         unsigned long flags;
 
-        orb->rcode = rcode;
-        if (rcode != RCODE_COMPLETE) {
-                spin_lock_irqsave(&card->lock, flags);
+        /*
+         * This is a little tricky.  We can get the status write for
+         * the orb before we get this callback.  The status write
+         * handler above will assume the orb pointer transaction was
+         * successful and set the rcode to RCODE_COMPLETE for the orb.
+         * So this callback only sets the rcode if it hasn't already
+         * been set and only does the cleanup if the transaction
+         * failed and we didn't already get a status write.
+         */
+        spin_lock_irqsave(&card->lock, flags);
+
+        if (orb->rcode == -1)
+                orb->rcode = rcode;
+        if (orb->rcode != RCODE_COMPLETE) {
                 list_del(&orb->link);
-                spin_unlock_irqrestore(&card->lock, flags);
                 orb->callback(orb, NULL);
         }
+
+        spin_unlock_irqrestore(&card->lock, flags);
+
+        kref_put(&orb->kref, free_orb);
 }
 
 static void
@@ -360,6 +385,10 @@ sbp2_send_orb(struct sbp2_orb *orb, struct fw_unit *unit,
         list_add_tail(&orb->link, &sd->orb_list);
         spin_unlock_irqrestore(&device->card->lock, flags);
 
+        /* Take a ref for the orb list and for the transaction callback. */
+        kref_get(&orb->kref);
+        kref_get(&orb->kref);
+
         fw_send_request(device->card, &orb->t, TCODE_WRITE_BLOCK_REQUEST,
                         node_id, generation, device->max_speed, offset,
                         &orb->pointer, sizeof(orb->pointer),
@@ -416,6 +445,7 @@ sbp2_send_management_orb(struct fw_unit *unit, int node_id, int generation,
         if (orb == NULL)
                 return -ENOMEM;
 
+        kref_init(&orb->base.kref);
         orb->response_bus =
                 dma_map_single(device->card->device, &orb->response,
                                sizeof(orb->response), DMA_FROM_DEVICE);
@@ -490,7 +520,7 @@ sbp2_send_management_orb(struct fw_unit *unit, int node_id, int generation,
         if (response)
                 fw_memcpy_from_be32(response,
                                     orb->response, sizeof(orb->response));
-        kfree(orb);
+        kref_put(&orb->base.kref, free_orb);
 
         return retval;
 }
@@ -886,7 +916,6 @@ complete_command_orb(struct sbp2_orb *base_orb, struct sbp2_status *status)
 
         orb->cmd->result = result;
         orb->done(orb->cmd);
-        kfree(orb);
 }
 
 static int sbp2_command_orb_map_scatterlist(struct sbp2_command_orb *orb)
@@ -1005,6 +1034,7 @@ static int sbp2_scsi_queuecommand(struct scsi_cmnd *cmd, scsi_done_fn_t done)
 
         /* Initialize rcode to something not RCODE_COMPLETE. */
         orb->base.rcode = -1;
+        kref_init(&orb->base.kref);
 
         orb->unit = unit;
         orb->done = done;
@@ -1051,10 +1081,11 @@ static int sbp2_scsi_queuecommand(struct scsi_cmnd *cmd, scsi_done_fn_t done)
         sbp2_send_orb(&orb->base, unit, sd->node_id, sd->generation,
                       sd->command_block_agent_address + SBP2_ORB_POINTER);
 
+        kref_put(&orb->base.kref, free_orb);
         return 0;
 
  fail_mapping:
-        kfree(orb);
+        kref_put(&orb->base.kref, free_orb);
  fail_alloc:
         return SCSI_MLQUEUE_HOST_BUSY;
 }