scsi: ch: fixup refcounting imbalance for SCSI devices
authorHannes Reinecke <hare@suse.de>
Thu, 13 Feb 2020 15:32:05 +0000 (16:32 +0100)
committerMartin K. Petersen <martin.petersen@oracle.com>
Mon, 24 Feb 2020 19:54:24 +0000 (14:54 -0500)
The SCSI device is required to be present during ch_probe() and ch_open().
But the SCSI device itself is only checked during ch_open(), so it's anyones
guess if it had been present during ch_probe(). And consequently we can't
reliably detach it during ch_release(), as ch_remove() might have been
called first.  So initialize the changer device during ch_probe(), and take
a reference to the SCSI device during both ch_probe() and ch_open().

[mkp: fixed checkpatch warning]

Link: https://lore.kernel.org/r/20200213153207.123357-2-hare@suse.de
Reviewed-by: Bart Van Assche <bvanassche@acm.org>
Signed-off-by: Hannes Reinecke <hare@suse.de>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
drivers/scsi/ch.c

index ed5f4a6..e3a006d 100644 (file)
@@ -569,6 +569,7 @@ static void ch_destroy(struct kref *ref)
 {
        scsi_changer *ch = container_of(ref, scsi_changer, ref);
 
+       ch->device = NULL;
        kfree(ch->dt);
        kfree(ch);
 }
@@ -594,14 +595,17 @@ ch_open(struct inode *inode, struct file *file)
        spin_lock(&ch_index_lock);
        ch = idr_find(&ch_index_idr, minor);
 
-       if (NULL == ch || scsi_device_get(ch->device)) {
+       if (ch == NULL || !kref_get_unless_zero(&ch->ref)) {
                spin_unlock(&ch_index_lock);
                mutex_unlock(&ch_mutex);
                return -ENXIO;
        }
-       kref_get(&ch->ref);
        spin_unlock(&ch_index_lock);
-
+       if (scsi_device_get(ch->device)) {
+               kref_put(&ch->ref, ch_destroy);
+               mutex_unlock(&ch_mutex);
+               return -ENXIO;
+       }
        file->private_data = ch;
        mutex_unlock(&ch_mutex);
        return 0;
@@ -938,6 +942,12 @@ static int ch_probe(struct device *dev)
 
        ch->minor = ret;
        sprintf(ch->name,"ch%d",ch->minor);
+       ret = scsi_device_get(sd);
+       if (ret) {
+               sdev_printk(KERN_WARNING, sd, "ch%d: failed to get device\n",
+                           ch->minor);
+               goto remove_idr;
+       }
 
        class_dev = device_create(ch_sysfs_class, dev,
                                  MKDEV(SCSI_CHANGER_MAJOR, ch->minor), ch,
@@ -946,7 +956,7 @@ static int ch_probe(struct device *dev)
                sdev_printk(KERN_WARNING, sd, "ch%d: device_create failed\n",
                            ch->minor);
                ret = PTR_ERR(class_dev);
-               goto remove_idr;
+               goto put_device;
        }
 
        mutex_init(&ch->lock);
@@ -964,6 +974,8 @@ static int ch_probe(struct device *dev)
        return 0;
 destroy_dev:
        device_destroy(ch_sysfs_class, MKDEV(SCSI_CHANGER_MAJOR, ch->minor));
+put_device:
+       scsi_device_put(sd);
 remove_idr:
        idr_remove(&ch_index_idr, ch->minor);
 free_ch:
@@ -977,9 +989,11 @@ static int ch_remove(struct device *dev)
 
        spin_lock(&ch_index_lock);
        idr_remove(&ch_index_idr, ch->minor);
+       dev_set_drvdata(dev, NULL);
        spin_unlock(&ch_index_lock);
 
        device_destroy(ch_sysfs_class, MKDEV(SCSI_CHANGER_MAJOR,ch->minor));
+       scsi_device_put(ch->device);
        kref_put(&ch->ref, ch_destroy);
        return 0;
 }