Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

[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>

authored by

James Bottomley and committed by
James Bottomley
884d25cc 26dacd0c

+16 -4
+16 -4
drivers/scsi/scsi_scan.c
··· 266 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 281 static struct scsi_target *scsi_alloc_target(struct device *parent, 270 282 int channel, uint id) 271 283 { ··· 343 331 return NULL; 344 332 } 345 333 } 334 + get_device(dev); 346 335 347 336 return starget; 348 337 349 338 found: 350 339 found_target->reap_ref++; 351 340 spin_unlock_irqrestore(shost->host_lock, flags); 352 - put_device(parent); 353 341 if (found_target->state != STARGET_DEL) { 342 + put_device(parent); 354 343 kfree(starget); 355 344 return found_target; 356 345 } ··· 1354 1341 if (!starget) 1355 1342 return ERR_PTR(-ENOMEM); 1356 1343 1357 - get_device(&starget->dev); 1358 1344 mutex_lock(&shost->scan_mutex); 1359 1345 if (scsi_host_scan_allowed(shost)) 1360 1346 scsi_probe_and_add_lun(starget, lun, NULL, &sdev, 1, hostdata); ··· 1412 1400 if (!starget) 1413 1401 return; 1414 1402 1415 - get_device(&starget->dev); 1416 1403 if (lun != SCAN_WILD_CARD) { 1417 1404 /* 1418 1405 * Scan for a specific host/chan/id/lun. ··· 1593 1582 if (sdev) { 1594 1583 sdev->sdev_gendev.parent = get_device(&starget->dev); 1595 1584 sdev->borken = 0; 1596 - } 1585 + } else 1586 + scsi_target_reap(starget); 1597 1587 put_device(&starget->dev); 1598 1588 out: 1599 1589 mutex_unlock(&shost->scan_mutex);