Skip to content

Commit

Permalink
firewire: sbp2: fix memory leak in sbp2_cancel_orbs or at send error
Browse files Browse the repository at this point in the history
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: Stefan Richter <stefanr@s5r6.in-berlin.de>
  • Loading branch information
Stefan Richter committed Aug 19, 2010
1 parent 840fe63 commit 6c74340
Showing 1 changed file with 7 additions and 5 deletions.
12 changes: 7 additions & 5 deletions drivers/firewire/sbp2.c
Original file line number Diff line number Diff line change
Expand Up @@ -450,7 +450,7 @@ static void sbp2_status_write(struct fw_card *card, struct fw_request *request,

if (&orb->link != &lu->orb_list) {
orb->callback(orb, &status);
kref_put(&orb->kref, free_orb);
kref_put(&orb->kref, free_orb); /* orb callback reference */
} else {
fw_error("status write for unknown orb\n");
}
Expand Down Expand Up @@ -480,12 +480,14 @@ static void complete_transaction(struct fw_card *card, int rcode,
if (orb->rcode != RCODE_COMPLETE) {
list_del(&orb->link);
spin_unlock_irqrestore(&card->lock, flags);

orb->callback(orb, NULL);
kref_put(&orb->kref, free_orb); /* orb callback reference */
} else {
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,
Expand All @@ -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);
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);
kref_get(&orb->kref); /* transaction callback reference */
kref_get(&orb->kref); /* orb callback reference */

fw_send_request(device->card, &orb->t, TCODE_WRITE_BLOCK_REQUEST,
node_id, generation, device->max_speed, offset,
Expand All @@ -530,6 +531,7 @@ static int sbp2_cancel_orbs(struct sbp2_logical_unit *lu)

orb->rcode = RCODE_CANCELLED;
orb->callback(orb, NULL);
kref_put(&orb->kref, free_orb); /* orb callback reference */
}

return retval;
Expand Down

0 comments on commit 6c74340

Please sign in to comment.