On 05/03/13 16:57, Joe Lawrence wrote: > Changes from v1: > Corrected error paths as noted by Ewan Milne and Jan Vesely. thanks, Acked-by: Jan Vesely <jvesely@xxxxxxxxxx> > > These changes were applied to scsi.git, branch "misc". This patch > fixes a reference count bug in the SCSI tape driver which can be > reproduced with the following: > > * Boot with slub_debug=FZPU, tape drive attached > * echo 1 > /sys/devices/... tape device pci path .../remove > * Wait for device removal > * echo 1 > /sys/kernel/slab/blkdev_queue/validate > * Slub debug complains about corrupted poison pattern > > In commit 523e1d39 (block: make gendisk hold a reference to its queue) > add_disk() and disk_release() were modified to get/put an additional > reference on a disk queue to fix a reference counting discrepency > between bdev release and SCSI device removal. The ST driver never > calls add_disk(), so this commit introduced an extra kref put when the > ST driver frees its struct gendisk. > > Attempts were made to fix this bug at the block level [1] but later > abandoned due to floppy driver issues [2]. > > [1] https://lkml.org/lkml/2012/8/27/354 > [2] https://lkml.org/lkml/2012/9/22/113 > > From a50a6ee28748b7c1620af6f76772164ec0fc4a1d Mon Sep 17 00:00:00 2001 > From: Joe Lawrence <joe.lawrence@xxxxxxxxxxx> > Date: Tue, 5 Mar 2013 09:30:14 -0500 > Subject: [PATCH v2] st: Take additional queue ref in st_probe > MIME-Version: 1.0 > Content-Type: text/plain; charset=UTF-8 > Content-Transfer-Encoding: 8bit > > The SCSI tape driver employs a struct gendisk, calling alloc_disk() to > create an instance, but does not register it via add_disk(). When the > gendisk is torn down, disk_release() is called and expects to return a > disk queue reference that add_disk() normally would have taken out. (See > commit 523e1d39.) Fix the kref accounting by adding a blk_get_queue() > to st_probe(). > > Signed-off-by: Joe Lawrence <joe.lawrence@xxxxxxxxxxx> > Tested-by: Ewan D. Milne <emilne@xxxxxxxxxx> > Cc: Kai Mäkisara <Kai.Makisara@xxxxxxxxxxx> > Cc: James E.J. Bottomley <JBottomley@xxxxxxxxxxxxx> > Cc: "Seymour, Shane M" <shane.seymour@xxxxxx> > Cc: Jan Vesely <jvesely@xxxxxxxxxx> > Cc: linux-scsi@xxxxxxxxxxxxxxx > --- > drivers/scsi/st.c | 10 ++++++++-- > 1 file changed, 8 insertions(+), 2 deletions(-) > > diff --git a/drivers/scsi/st.c b/drivers/scsi/st.c > index 98156a9..96f3363 100644 > --- a/drivers/scsi/st.c > +++ b/drivers/scsi/st.c > @@ -4112,6 +4112,10 @@ static int st_probe(struct device *dev) > tpnt->disk = disk; > disk->private_data = &tpnt->driver; > disk->queue = SDp->request_queue; > + /* SCSI tape doesn't register this gendisk via add_disk(). Manually > + * take queue reference that release_disk() expects. */ > + if (!blk_get_queue(disk->queue)) > + goto out_put_disk; > tpnt->driver = &st_template; > > tpnt->device = SDp; > @@ -4181,7 +4185,7 @@ static int st_probe(struct device *dev) > if (!idr_pre_get(&st_index_idr, GFP_KERNEL)) { > pr_warn("st: idr expansion failed\n"); > error = -ENOMEM; > - goto out_put_disk; > + goto out_put_queue; > } > > spin_lock(&st_index_lock); > @@ -4189,7 +4193,7 @@ static int st_probe(struct device *dev) > spin_unlock(&st_index_lock); > if (error) { > pr_warn("st: idr allocation failed: %d\n", error); > - goto out_put_disk; > + goto out_put_queue; > } > > if (dev_num > ST_MAX_TAPES) { > @@ -4222,6 +4226,8 @@ out_put_index: > spin_lock(&st_index_lock); > idr_remove(&st_index_idr, dev_num); > spin_unlock(&st_index_lock); > +out_put_queue: > + blk_put_queue(disk->queue); > out_put_disk: > put_disk(disk); > kfree(tpnt); > -- Jan Vesely <jvesely@xxxxxxxxxx> -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
