From mboxrd@z Thu Jan 1 00:00:00 1970 From: James Bottomley Subject: [RFC 2/2] dual scan thread bug fix Date: Mon, 16 Dec 2013 07:13:41 -0800 Message-ID: <1387206821.2200.8.camel@dabdike.int.hansenpartnership.com> References: <1387206619.2200.6.camel@dabdike.int.hansenpartnership.com> Mime-Version: 1.0 Content-Type: text/plain; charset="ISO-8859-15" Content-Transfer-Encoding: 7bit Return-path: Received: from bedivere.hansenpartnership.com ([66.63.167.143]:33517 "EHLO bedivere.hansenpartnership.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753978Ab3LPPNn (ORCPT ); Mon, 16 Dec 2013 10:13:43 -0500 In-Reply-To: <1387206619.2200.6.camel@dabdike.int.hansenpartnership.com> Sender: linux-scsi-owner@vger.kernel.org List-Id: linux-scsi@vger.kernel.org To: linux-scsi Cc: Alan Stern , USB list 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. --- drivers/scsi/scsi_scan.c | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c index 39e5c85..2d7aafa 100644 --- a/drivers/scsi/scsi_scan.c +++ b/drivers/scsi/scsi_scan.c @@ -320,6 +320,7 @@ static void scsi_target_destroy(struct scsi_target *starget) struct Scsi_Host *shost = dev_to_shost(dev->parent); unsigned long flags; + starget->state = STARGET_DEL; transport_destroy_device(dev); spin_lock_irqsave(shost->host_lock, flags); if (shost->hostt->target_destroy) @@ -384,9 +385,15 @@ static void scsi_target_reap_ref_release(struct kref *kref) struct scsi_target *starget = container_of(kref, struct scsi_target, reap_ref); - transport_remove_device(&starget->dev); - device_del(&starget->dev); - starget->state = STARGET_DEL; + /* + * if we get here and the target is still in the CREATED state that + * means it was allocated but never made visible (because a scan + * turned up no LUNs), so don't call device_del() on it. + */ + if (starget->state == STARGET_RUNNING) { + transport_remove_device(&starget->dev); + device_del(&starget->dev); + } scsi_target_destroy(starget); } @@ -502,11 +509,13 @@ static struct scsi_target *scsi_alloc_target(struct device *parent, */ void scsi_target_reap(struct scsi_target *starget) { + /* + * serious problem if this triggers: STARGET_DEL is only set in the + * kref release routine, so we're doing another final put on an + * already released kref + */ BUG_ON(starget->state == STARGET_DEL); - if (starget->state == STARGET_CREATED) - scsi_target_destroy(starget); - else - scsi_target_reap_ref_put(starget); + scsi_target_reap_ref_put(starget); } /** -- 1.8.4.3