Skip to content

Commit f2495e2

Browse files
James BottomleyJames Bottomley
authored andcommitted
[SCSI] dual scan thread bug fix
In the highly unusual case where two threads are running concurrently through the scanning code scanning the same target, we run into the situation where one may allocate the target while the other is still using it. In this case, because the reap checks for STARGET_CREATED and kills the target without reference counting, the second thread will do the wrong thing on reap. Fix this by reference counting even creates and doing the STARGET_CREATED check in the final put. Tested-by: Sarah Sharp <[email protected]> Cc: [email protected] # delay backport for 2 months for field testing Signed-off-by: James Bottomley <[email protected]>
1 parent e63ed0d commit f2495e2

File tree

1 file changed

+16
-7
lines changed

1 file changed

+16
-7
lines changed

drivers/scsi/scsi_scan.c

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -320,6 +320,7 @@ static void scsi_target_destroy(struct scsi_target *starget)
320320
struct Scsi_Host *shost = dev_to_shost(dev->parent);
321321
unsigned long flags;
322322

323+
starget->state = STARGET_DEL;
323324
transport_destroy_device(dev);
324325
spin_lock_irqsave(shost->host_lock, flags);
325326
if (shost->hostt->target_destroy)
@@ -384,9 +385,15 @@ static void scsi_target_reap_ref_release(struct kref *kref)
384385
struct scsi_target *starget
385386
= container_of(kref, struct scsi_target, reap_ref);
386387

387-
transport_remove_device(&starget->dev);
388-
device_del(&starget->dev);
389-
starget->state = STARGET_DEL;
388+
/*
389+
* if we get here and the target is still in the CREATED state that
390+
* means it was allocated but never made visible (because a scan
391+
* turned up no LUNs), so don't call device_del() on it.
392+
*/
393+
if (starget->state != STARGET_CREATED) {
394+
transport_remove_device(&starget->dev);
395+
device_del(&starget->dev);
396+
}
390397
scsi_target_destroy(starget);
391398
}
392399

@@ -506,11 +513,13 @@ static struct scsi_target *scsi_alloc_target(struct device *parent,
506513
*/
507514
void scsi_target_reap(struct scsi_target *starget)
508515
{
516+
/*
517+
* serious problem if this triggers: STARGET_DEL is only set in the if
518+
* the reap_ref drops to zero, so we're trying to do another final put
519+
* on an already released kref
520+
*/
509521
BUG_ON(starget->state == STARGET_DEL);
510-
if (starget->state == STARGET_CREATED)
511-
scsi_target_destroy(starget);
512-
else
513-
scsi_target_reap_ref_put(starget);
522+
scsi_target_reap_ref_put(starget);
514523
}
515524

516525
/**

0 commit comments

Comments
 (0)