Commit 6c74340b authored by Stefan Richter's avatar Stefan Richter

firewire: sbp2: fix memory leak in sbp2_cancel_orbs or at send error

When an ORB was canceled (Command ORB i.e. SCSI request timed out, or
Management ORB timed out), or there was a send error in the initial
transaction, we missed to drop one of the ORB's references and thus
leaked memory.

Background:
In total, we hold 3 references to each Operation Request Block:
  - 1 during sbp2_scsi_queuecommand() or sbp2_send_management_orb()
    respectively,
  - 1 for the duration of the write transaction to the ORB_Pointer or
    Management_Agent register of the target,
  - 1 for as long as the ORB stays within the lu->orb_list, until
    the ORB is unlinked from the list and the orb->callback was
    executed.

The latter one of these 3 references is finished
  - normally by sbp2_status_write() when the target wrote status
    for a pending ORB,
  - or by sbp2_cancel_orbs() in case of an ORB time-out,
  - or by complete_transaction() in case of a send error.
Of them, the latter two lacked the kref_put.

Add the missing kref_put()s.  Add comments to the gets and puts of
references for transaction callbacks and ORB callbacks so that it is
easier to see what is supposed to happen.
Signed-off-by: default avatarStefan Richter <stefanr@s5r6.in-berlin.de>
parent 840fe635
...@@ -450,7 +450,7 @@ static void sbp2_status_write(struct fw_card *card, struct fw_request *request, ...@@ -450,7 +450,7 @@ static void sbp2_status_write(struct fw_card *card, struct fw_request *request,
if (&orb->link != &lu->orb_list) { if (&orb->link != &lu->orb_list) {
orb->callback(orb, &status); orb->callback(orb, &status);
kref_put(&orb->kref, free_orb); kref_put(&orb->kref, free_orb); /* orb callback reference */
} else { } else {
fw_error("status write for unknown orb\n"); fw_error("status write for unknown orb\n");
} }
...@@ -480,12 +480,14 @@ static void complete_transaction(struct fw_card *card, int rcode, ...@@ -480,12 +480,14 @@ static void complete_transaction(struct fw_card *card, int rcode,
if (orb->rcode != RCODE_COMPLETE) { if (orb->rcode != RCODE_COMPLETE) {
list_del(&orb->link); list_del(&orb->link);
spin_unlock_irqrestore(&card->lock, flags); spin_unlock_irqrestore(&card->lock, flags);
orb->callback(orb, NULL); orb->callback(orb, NULL);
kref_put(&orb->kref, free_orb); /* orb callback reference */
} else { } else {
spin_unlock_irqrestore(&card->lock, flags); spin_unlock_irqrestore(&card->lock, flags);
} }
kref_put(&orb->kref, free_orb); kref_put(&orb->kref, free_orb); /* transaction callback reference */
} }
static void sbp2_send_orb(struct sbp2_orb *orb, struct sbp2_logical_unit *lu, static void sbp2_send_orb(struct sbp2_orb *orb, struct sbp2_logical_unit *lu,
...@@ -501,9 +503,8 @@ static void sbp2_send_orb(struct sbp2_orb *orb, struct sbp2_logical_unit *lu, ...@@ -501,9 +503,8 @@ static void sbp2_send_orb(struct sbp2_orb *orb, struct sbp2_logical_unit *lu,
list_add_tail(&orb->link, &lu->orb_list); list_add_tail(&orb->link, &lu->orb_list);
spin_unlock_irqrestore(&device->card->lock, flags); spin_unlock_irqrestore(&device->card->lock, flags);
/* Take a ref for the orb list and for the transaction callback. */ kref_get(&orb->kref); /* transaction callback reference */
kref_get(&orb->kref); kref_get(&orb->kref); /* orb callback reference */
kref_get(&orb->kref);
fw_send_request(device->card, &orb->t, TCODE_WRITE_BLOCK_REQUEST, fw_send_request(device->card, &orb->t, TCODE_WRITE_BLOCK_REQUEST,
node_id, generation, device->max_speed, offset, node_id, generation, device->max_speed, offset,
...@@ -530,6 +531,7 @@ static int sbp2_cancel_orbs(struct sbp2_logical_unit *lu) ...@@ -530,6 +531,7 @@ static int sbp2_cancel_orbs(struct sbp2_logical_unit *lu)
orb->rcode = RCODE_CANCELLED; orb->rcode = RCODE_CANCELLED;
orb->callback(orb, NULL); orb->callback(orb, NULL);
kref_put(&orb->kref, free_orb); /* orb callback reference */
} }
return retval; return retval;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment