Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756238Ab1E0UVZ (ORCPT ); Fri, 27 May 2011 16:21:25 -0400 Received: from bedivere.hansenpartnership.com ([66.63.167.143]:42005 "EHLO bedivere.hansenpartnership.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752287Ab1E0UVV (ORCPT ); Fri, 27 May 2011 16:21:21 -0400 Subject: Re: [PATCH] SCSI IOCTL: Check for device deletion [was Re: __elv_add_request OOPS] From: James Bottomley To: Jens Axboe Cc: Parag Warudkar , Linus Torvalds , "linux-kernel@vger.kernel.org" , "akpm@linux-foundation.org" , Linux SCSI List In-Reply-To: <4DDF3A07.7040106@fusionio.com> References: <4DDB8BF6.2000304@fusionio.com> <4DDCB1C8.7040708@fusionio.com> <4DDD5240.2060308@fusionio.com> <4DDD55D6.1080909@fusionio.com> <1306356735.1641.61.camel@mulgrave.site> <1306468385.4048.73.camel@mulgrave.site> <4DDF3A07.7040106@fusionio.com> Content-Type: text/plain; charset="UTF-8" Date: Fri, 27 May 2011 15:21:17 -0500 Message-ID: <1306527677.12244.2.camel@mulgrave.site> Mime-Version: 1.0 X-Mailer: Evolution 2.32.1 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3984 Lines: 117 On Fri, 2011-05-27 at 07:43 +0200, Jens Axboe wrote: > On 2011-05-27 05:53, James Bottomley wrote: > > On Wed, 2011-05-25 at 15:52 -0500, James Bottomley wrote: > >> On Wed, 2011-05-25 at 16:42 -0400, Parag Warudkar wrote: > >>> Yeah - that makes sense. By that logic, looks like we can only disallow > >>> for SDEV_DEL (if we decide to do that check here). > >> > >> I don't think this is the root cause. I think q is non-NULL but has > >> already been released, so we're just getting a NULL deref on the actual > >> list head. > >> > >> Does this fix it? It adds the refcounting at approximately the correct > >> places. Of course, we'll now be trying elevator operations on an extant > >> queue but one whose elevator functions have been destroyed, so there are > >> probably additional state guards to place. > >> > >> James > >> > >> --- > >> > >> diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c > >> index 58584dc..44e8ca3 100644 > >> --- a/drivers/scsi/scsi_scan.c > >> +++ b/drivers/scsi/scsi_scan.c > >> @@ -297,7 +297,7 @@ static struct scsi_device *scsi_alloc_sdev(struct scsi_target *starget, > >> kfree(sdev); > >> goto out; > >> } > >> - > >> + blk_get_queue(sdev->request_queue); > > > > Apparently we can't do this because blk_[put|get]_queue aren't exported > > for use in modules (and SCSI can be modular), so this caused a build > > failure in linux-next. > > I'll just export them. OK, if you're fine with that, how does this patch look? I can take it through my rc-fixes tree (with your ack), since I'm a bit late on the merge window now. James --- From: James Bottomley Subject: [PATCH] [SCSI] Fix oops caused by queue refcounting failure In certain circumstances, we can get an oops from a torn down device. Most notably this is from CD roms trying to call scsi_ioctl. The root cause of the problem is the fact that after scsi_remove_device() has been called, the queue is fully torn down. This is actually wrong since the queue can be used until the sdev release function is called. Therefore, we add an extra reference to the queue which is released in sdev->release, so the queue always exists. Also export blk_queue_get/blk_queue_put for the refcounting. Reported-by: Parag Warudkar Signed-off-by: James Bottomley diff --git a/block/blk-core.c b/block/blk-core.c index 3fe00a1..1aa776c 100644 --- a/block/blk-core.c +++ b/block/blk-core.c @@ -345,6 +345,7 @@ void blk_put_queue(struct request_queue *q) { kobject_put(&q->kobj); } +EXPORT_SYMBOL_GPL(blk_put_queue); /* * Note: If a driver supplied the queue lock, it should not zap that lock @@ -566,6 +567,7 @@ int blk_get_queue(struct request_queue *q) return 1; } +EXPORT_SYMBOL_GPL(blk_get_queue); static inline void blk_free_request(struct request_queue *q, struct request *rq) { diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c index 58584dc..44e8ca3 100644 --- a/drivers/scsi/scsi_scan.c +++ b/drivers/scsi/scsi_scan.c @@ -297,7 +297,7 @@ static struct scsi_device *scsi_alloc_sdev(struct scsi_target *starget, kfree(sdev); goto out; } - + blk_get_queue(sdev->request_queue); sdev->request_queue->queuedata = sdev; scsi_adjust_queue_depth(sdev, 0, sdev->host->cmd_per_lun); diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c index e639125..e0bd3f7 100644 --- a/drivers/scsi/scsi_sysfs.c +++ b/drivers/scsi/scsi_sysfs.c @@ -322,6 +322,7 @@ static void scsi_device_dev_release_usercontext(struct work_struct *work) kfree(evt); } + blk_put_queue(sdev->request_queue); /* NULL queue means the device can't be used */ sdev->request_queue = NULL; -- 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/