diff options
author | James Bottomley <James.Bottomley@steeleye.com> | 2006-09-05 17:26:41 -0400 |
---|---|---|
committer | James Bottomley <jejb@mulgrave.il.steeleye.com> | 2006-09-07 11:08:43 -0400 |
commit | 884d25cc4fda20908fd4ef93dbb41d817984b68b (patch) | |
tree | 99a7a2a02ff76bee9c4a0620e6a90321517bba13 | |
parent | 26dacd0c9b2dc1dc987c376aeee4e80691a7dd0b (diff) |
[SCSI] Fix refcount breakage with 'echo "1" > scan' when target already present
Spotted by: Dan Aloni <da-xx@monatomic.org>
The problem is there's inconsistent locking semantic usage of
scsi_alloc_target(). Two callers assume the target comes back with
reference unincremented and the third assumes its incremented. Fix by
always making the reference incremented on return. Also fix path in
target alloc that could consistently increment the parent lock.
Finally document scsi_alloc_target() so its callers know what the
expectations are.
Signed-off-by: James Bottomley <James.Bottomley@SteelEye.com>
-rw-r--r-- | drivers/scsi/scsi_scan.c | 20 |
1 files changed, 16 insertions, 4 deletions
diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c index 31d05ab0b2fc..fd9e281c3bfe 100644 --- a/drivers/scsi/scsi_scan.c +++ b/drivers/scsi/scsi_scan.c | |||
@@ -266,6 +266,18 @@ static struct scsi_target *__scsi_find_target(struct device *parent, | |||
266 | return found_starget; | 266 | return found_starget; |
267 | } | 267 | } |
268 | 268 | ||
269 | /** | ||
270 | * scsi_alloc_target - allocate a new or find an existing target | ||
271 | * @parent: parent of the target (need not be a scsi host) | ||
272 | * @channel: target channel number (zero if no channels) | ||
273 | * @id: target id number | ||
274 | * | ||
275 | * Return an existing target if one exists, provided it hasn't already | ||
276 | * gone into STARGET_DEL state, otherwise allocate a new target. | ||
277 | * | ||
278 | * The target is returned with an incremented reference, so the caller | ||
279 | * is responsible for both reaping and doing a last put | ||
280 | */ | ||
269 | static struct scsi_target *scsi_alloc_target(struct device *parent, | 281 | static struct scsi_target *scsi_alloc_target(struct device *parent, |
270 | int channel, uint id) | 282 | int channel, uint id) |
271 | { | 283 | { |
@@ -331,14 +343,15 @@ static struct scsi_target *scsi_alloc_target(struct device *parent, | |||
331 | return NULL; | 343 | return NULL; |
332 | } | 344 | } |
333 | } | 345 | } |
346 | get_device(dev); | ||
334 | 347 | ||
335 | return starget; | 348 | return starget; |
336 | 349 | ||
337 | found: | 350 | found: |
338 | found_target->reap_ref++; | 351 | found_target->reap_ref++; |
339 | spin_unlock_irqrestore(shost->host_lock, flags); | 352 | spin_unlock_irqrestore(shost->host_lock, flags); |
340 | put_device(parent); | ||
341 | if (found_target->state != STARGET_DEL) { | 353 | if (found_target->state != STARGET_DEL) { |
354 | put_device(parent); | ||
342 | kfree(starget); | 355 | kfree(starget); |
343 | return found_target; | 356 | return found_target; |
344 | } | 357 | } |
@@ -1341,7 +1354,6 @@ struct scsi_device *__scsi_add_device(struct Scsi_Host *shost, uint channel, | |||
1341 | if (!starget) | 1354 | if (!starget) |
1342 | return ERR_PTR(-ENOMEM); | 1355 | return ERR_PTR(-ENOMEM); |
1343 | 1356 | ||
1344 | get_device(&starget->dev); | ||
1345 | mutex_lock(&shost->scan_mutex); | 1357 | mutex_lock(&shost->scan_mutex); |
1346 | if (scsi_host_scan_allowed(shost)) | 1358 | if (scsi_host_scan_allowed(shost)) |
1347 | scsi_probe_and_add_lun(starget, lun, NULL, &sdev, 1, hostdata); | 1359 | scsi_probe_and_add_lun(starget, lun, NULL, &sdev, 1, hostdata); |
@@ -1400,7 +1412,6 @@ static void __scsi_scan_target(struct device *parent, unsigned int channel, | |||
1400 | if (!starget) | 1412 | if (!starget) |
1401 | return; | 1413 | return; |
1402 | 1414 | ||
1403 | get_device(&starget->dev); | ||
1404 | if (lun != SCAN_WILD_CARD) { | 1415 | if (lun != SCAN_WILD_CARD) { |
1405 | /* | 1416 | /* |
1406 | * Scan for a specific host/chan/id/lun. | 1417 | * Scan for a specific host/chan/id/lun. |
@@ -1582,7 +1593,8 @@ struct scsi_device *scsi_get_host_dev(struct Scsi_Host *shost) | |||
1582 | if (sdev) { | 1593 | if (sdev) { |
1583 | sdev->sdev_gendev.parent = get_device(&starget->dev); | 1594 | sdev->sdev_gendev.parent = get_device(&starget->dev); |
1584 | sdev->borken = 0; | 1595 | sdev->borken = 0; |
1585 | } | 1596 | } else |
1597 | scsi_target_reap(starget); | ||
1586 | put_device(&starget->dev); | 1598 | put_device(&starget->dev); |
1587 | out: | 1599 | out: |
1588 | mutex_unlock(&shost->scan_mutex); | 1600 | mutex_unlock(&shost->scan_mutex); |