Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1759730Ab2BNTLY (ORCPT ); Tue, 14 Feb 2012 14:11:24 -0500 Received: from relay01ant.iops.be ([212.53.4.34]:35943 "EHLO relay01ant.iops.be" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753215Ab2BNTLV (ORCPT ); Tue, 14 Feb 2012 14:11:21 -0500 From: Bart Van Assche To: Stefan Richter Cc: "Jun'ichi Nomura" , jbottomley@parallels.com, linux-scsi@vger.kernel.org, Huajun Li , Axel Theilmann , linux-kernel@vger.kernel.org Subject: Re: Re: Yet another hot unplug NULL pointer dereference (was Re: status of oops in sd_revalidate_disk?) Date: Tue, 14 Feb 2012 19:11:16 +0000 Message-ID: <2173642.Y3HxI15Hxl@asus> User-Agent: KMail/4.7.2 (Linux/3.2.6+; KDE/4.7.2; x86_64; ; ) In-Reply-To: <20120214143800.69703df9@stein> References: <4EE8E419.8010000@pre-sense.de> <20120214143800.69703df9@stein> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2819 Lines: 89 On Feb 14 Stefan Richter wrote: > Or asked differently, what is supposed to serialize the ->queuedata > accesses? How about avoiding to modify q->queuedata before scsi_free_queue() has been invoked, e.g. as follows ? Note: the kfree() call in scsi_host_dev_release() probably should be postponed until the last put on the queue has occurred. >From 4d29b62c686a718d34d3b3f634306376b40dbdb6 Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Tue, 14 Feb 2012 13:52:21 +0100 Subject: [PATCH] scsi-queuedata-null-deref-fix --- drivers/scsi/hosts.c | 4 ++-- drivers/scsi/scsi_lib.c | 13 +++---------- drivers/scsi/scsi_sysfs.c | 3 --- 3 files changed, 5 insertions(+), 15 deletions(-) diff --git a/drivers/scsi/hosts.c b/drivers/scsi/hosts.c index 351dc0b..5f34620 100644 --- a/drivers/scsi/hosts.c +++ b/drivers/scsi/hosts.c @@ -296,9 +296,9 @@ static void scsi_host_dev_release(struct device *dev) destroy_workqueue(shost->work_q); q = shost->uspace_req_q; if (q) { - kfree(q->queuedata); - q->queuedata = NULL; + void *qd = q->queuedata; scsi_free_queue(q); + kfree(qd); } scsi_destroy_command_freelist(shost); diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c index f85cfa6..7c40303 100644 --- a/drivers/scsi/scsi_lib.c +++ b/drivers/scsi/scsi_lib.c @@ -1491,7 +1491,9 @@ static void scsi_request_fn(struct request_queue *q) struct scsi_cmnd *cmd; struct request *req; - if (!sdev) { + BUG_ON(!sdev); + + if (test_bit(QUEUE_FLAG_DEAD, &q->queue_flags)) { while ((req = blk_peek_request(q)) != NULL) scsi_kill_request(req, q); return; @@ -1700,15 +1702,6 @@ struct request_queue *scsi_alloc_queue(struct scsi_device *sdev) void scsi_free_queue(struct request_queue *q) { - unsigned long flags; - - WARN_ON(q->queuedata); - - /* cause scsi_request_fn() to kill all non-finished requests */ - spin_lock_irqsave(q->queue_lock, flags); - q->request_fn(q); - spin_unlock_irqrestore(q->queue_lock, flags); - blk_cleanup_queue(q); } diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c index ea5658d..61dd107 100644 --- a/drivers/scsi/scsi_sysfs.c +++ b/drivers/scsi/scsi_sysfs.c @@ -976,9 +976,6 @@ void __scsi_remove_device(struct scsi_device *sdev) sdev->host->hostt->slave_destroy(sdev); transport_destroy_device(dev); - /* cause the request function to reject all I/O requests */ - sdev->request_queue->queuedata = NULL; - /* Freeing the queue signals to block that we're done */ scsi_free_queue(sdev->request_queue); put_device(dev); -- 1.7.7 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/