ide: fix refcounting in device drivers
During host driver module removal del_gendisk() results in a final put on drive->gendev and freeing the drive by drive_release_dev(). Convert device drivers from using struct kref to use struct device so device driver's object holds reference on ->gendev and prevents drive from prematurely going away. Also fix ->remove methods to not erroneously drop reference on a host driver by using only put_device() instead of ide*_put(). Reported-by: Stanislaw Gruszka <stf_xl@wp.pl> Tested-by: Stanislaw Gruszka <stf_xl@wp.pl> Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
This commit is contained in:
parent
d3dd7107f4
commit
8fed436841
6 changed files with 57 additions and 31 deletions
|
@ -55,7 +55,7 @@
|
||||||
|
|
||||||
static DEFINE_MUTEX(idecd_ref_mutex);
|
static DEFINE_MUTEX(idecd_ref_mutex);
|
||||||
|
|
||||||
static void ide_cd_release(struct kref *);
|
static void ide_cd_release(struct device *);
|
||||||
|
|
||||||
static struct cdrom_info *ide_cd_get(struct gendisk *disk)
|
static struct cdrom_info *ide_cd_get(struct gendisk *disk)
|
||||||
{
|
{
|
||||||
|
@ -67,7 +67,7 @@ static struct cdrom_info *ide_cd_get(struct gendisk *disk)
|
||||||
if (ide_device_get(cd->drive))
|
if (ide_device_get(cd->drive))
|
||||||
cd = NULL;
|
cd = NULL;
|
||||||
else
|
else
|
||||||
kref_get(&cd->kref);
|
get_device(&cd->dev);
|
||||||
|
|
||||||
}
|
}
|
||||||
mutex_unlock(&idecd_ref_mutex);
|
mutex_unlock(&idecd_ref_mutex);
|
||||||
|
@ -79,7 +79,7 @@ static void ide_cd_put(struct cdrom_info *cd)
|
||||||
ide_drive_t *drive = cd->drive;
|
ide_drive_t *drive = cd->drive;
|
||||||
|
|
||||||
mutex_lock(&idecd_ref_mutex);
|
mutex_lock(&idecd_ref_mutex);
|
||||||
kref_put(&cd->kref, ide_cd_release);
|
put_device(&cd->dev);
|
||||||
ide_device_put(drive);
|
ide_device_put(drive);
|
||||||
mutex_unlock(&idecd_ref_mutex);
|
mutex_unlock(&idecd_ref_mutex);
|
||||||
}
|
}
|
||||||
|
@ -1798,15 +1798,17 @@ static void ide_cd_remove(ide_drive_t *drive)
|
||||||
ide_debug_log(IDE_DBG_FUNC, "Call %s\n", __func__);
|
ide_debug_log(IDE_DBG_FUNC, "Call %s\n", __func__);
|
||||||
|
|
||||||
ide_proc_unregister_driver(drive, info->driver);
|
ide_proc_unregister_driver(drive, info->driver);
|
||||||
|
device_del(&info->dev);
|
||||||
del_gendisk(info->disk);
|
del_gendisk(info->disk);
|
||||||
|
|
||||||
ide_cd_put(info);
|
mutex_lock(&idecd_ref_mutex);
|
||||||
|
put_device(&info->dev);
|
||||||
|
mutex_unlock(&idecd_ref_mutex);
|
||||||
}
|
}
|
||||||
|
|
||||||
static void ide_cd_release(struct kref *kref)
|
static void ide_cd_release(struct device *dev)
|
||||||
{
|
{
|
||||||
struct cdrom_info *info = to_ide_drv(kref, cdrom_info);
|
struct cdrom_info *info = to_ide_drv(dev, cdrom_info);
|
||||||
struct cdrom_device_info *devinfo = &info->devinfo;
|
struct cdrom_device_info *devinfo = &info->devinfo;
|
||||||
ide_drive_t *drive = info->drive;
|
ide_drive_t *drive = info->drive;
|
||||||
struct gendisk *g = info->disk;
|
struct gendisk *g = info->disk;
|
||||||
|
@ -2005,7 +2007,12 @@ static int ide_cd_probe(ide_drive_t *drive)
|
||||||
|
|
||||||
ide_init_disk(g, drive);
|
ide_init_disk(g, drive);
|
||||||
|
|
||||||
kref_init(&info->kref);
|
info->dev.parent = &drive->gendev;
|
||||||
|
info->dev.release = ide_cd_release;
|
||||||
|
dev_set_name(&info->dev, dev_name(&drive->gendev));
|
||||||
|
|
||||||
|
if (device_register(&info->dev))
|
||||||
|
goto out_free_disk;
|
||||||
|
|
||||||
info->drive = drive;
|
info->drive = drive;
|
||||||
info->driver = &ide_cdrom_driver;
|
info->driver = &ide_cdrom_driver;
|
||||||
|
@ -2019,7 +2026,7 @@ static int ide_cd_probe(ide_drive_t *drive)
|
||||||
g->driverfs_dev = &drive->gendev;
|
g->driverfs_dev = &drive->gendev;
|
||||||
g->flags = GENHD_FL_CD | GENHD_FL_REMOVABLE;
|
g->flags = GENHD_FL_CD | GENHD_FL_REMOVABLE;
|
||||||
if (ide_cdrom_setup(drive)) {
|
if (ide_cdrom_setup(drive)) {
|
||||||
ide_cd_release(&info->kref);
|
put_device(&info->dev);
|
||||||
goto failed;
|
goto failed;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -2029,6 +2036,8 @@ static int ide_cd_probe(ide_drive_t *drive)
|
||||||
add_disk(g);
|
add_disk(g);
|
||||||
return 0;
|
return 0;
|
||||||
|
|
||||||
|
out_free_disk:
|
||||||
|
put_disk(g);
|
||||||
out_free_cd:
|
out_free_cd:
|
||||||
kfree(info);
|
kfree(info);
|
||||||
failed:
|
failed:
|
||||||
|
|
|
@ -80,7 +80,7 @@ struct cdrom_info {
|
||||||
ide_drive_t *drive;
|
ide_drive_t *drive;
|
||||||
struct ide_driver *driver;
|
struct ide_driver *driver;
|
||||||
struct gendisk *disk;
|
struct gendisk *disk;
|
||||||
struct kref kref;
|
struct device dev;
|
||||||
|
|
||||||
/* Buffer for table of contents. NULL if we haven't allocated
|
/* Buffer for table of contents. NULL if we haven't allocated
|
||||||
a TOC buffer for this device yet. */
|
a TOC buffer for this device yet. */
|
||||||
|
|
|
@ -25,7 +25,7 @@ module_param(debug_mask, ulong, 0644);
|
||||||
|
|
||||||
static DEFINE_MUTEX(ide_disk_ref_mutex);
|
static DEFINE_MUTEX(ide_disk_ref_mutex);
|
||||||
|
|
||||||
static void ide_disk_release(struct kref *);
|
static void ide_disk_release(struct device *);
|
||||||
|
|
||||||
static struct ide_disk_obj *ide_disk_get(struct gendisk *disk)
|
static struct ide_disk_obj *ide_disk_get(struct gendisk *disk)
|
||||||
{
|
{
|
||||||
|
@ -37,7 +37,7 @@ static struct ide_disk_obj *ide_disk_get(struct gendisk *disk)
|
||||||
if (ide_device_get(idkp->drive))
|
if (ide_device_get(idkp->drive))
|
||||||
idkp = NULL;
|
idkp = NULL;
|
||||||
else
|
else
|
||||||
kref_get(&idkp->kref);
|
get_device(&idkp->dev);
|
||||||
}
|
}
|
||||||
mutex_unlock(&ide_disk_ref_mutex);
|
mutex_unlock(&ide_disk_ref_mutex);
|
||||||
return idkp;
|
return idkp;
|
||||||
|
@ -48,7 +48,7 @@ static void ide_disk_put(struct ide_disk_obj *idkp)
|
||||||
ide_drive_t *drive = idkp->drive;
|
ide_drive_t *drive = idkp->drive;
|
||||||
|
|
||||||
mutex_lock(&ide_disk_ref_mutex);
|
mutex_lock(&ide_disk_ref_mutex);
|
||||||
kref_put(&idkp->kref, ide_disk_release);
|
put_device(&idkp->dev);
|
||||||
ide_device_put(drive);
|
ide_device_put(drive);
|
||||||
mutex_unlock(&ide_disk_ref_mutex);
|
mutex_unlock(&ide_disk_ref_mutex);
|
||||||
}
|
}
|
||||||
|
@ -66,17 +66,18 @@ static void ide_gd_remove(ide_drive_t *drive)
|
||||||
struct gendisk *g = idkp->disk;
|
struct gendisk *g = idkp->disk;
|
||||||
|
|
||||||
ide_proc_unregister_driver(drive, idkp->driver);
|
ide_proc_unregister_driver(drive, idkp->driver);
|
||||||
|
device_del(&idkp->dev);
|
||||||
del_gendisk(g);
|
del_gendisk(g);
|
||||||
|
|
||||||
drive->disk_ops->flush(drive);
|
drive->disk_ops->flush(drive);
|
||||||
|
|
||||||
ide_disk_put(idkp);
|
mutex_lock(&ide_disk_ref_mutex);
|
||||||
|
put_device(&idkp->dev);
|
||||||
|
mutex_unlock(&ide_disk_ref_mutex);
|
||||||
}
|
}
|
||||||
|
|
||||||
static void ide_disk_release(struct kref *kref)
|
static void ide_disk_release(struct device *dev)
|
||||||
{
|
{
|
||||||
struct ide_disk_obj *idkp = to_ide_drv(kref, ide_disk_obj);
|
struct ide_disk_obj *idkp = to_ide_drv(dev, ide_disk_obj);
|
||||||
ide_drive_t *drive = idkp->drive;
|
ide_drive_t *drive = idkp->drive;
|
||||||
struct gendisk *g = idkp->disk;
|
struct gendisk *g = idkp->disk;
|
||||||
|
|
||||||
|
@ -348,7 +349,12 @@ static int ide_gd_probe(ide_drive_t *drive)
|
||||||
|
|
||||||
ide_init_disk(g, drive);
|
ide_init_disk(g, drive);
|
||||||
|
|
||||||
kref_init(&idkp->kref);
|
idkp->dev.parent = &drive->gendev;
|
||||||
|
idkp->dev.release = ide_disk_release;
|
||||||
|
dev_set_name(&idkp->dev, dev_name(&drive->gendev));
|
||||||
|
|
||||||
|
if (device_register(&idkp->dev))
|
||||||
|
goto out_free_disk;
|
||||||
|
|
||||||
idkp->drive = drive;
|
idkp->drive = drive;
|
||||||
idkp->driver = &ide_gd_driver;
|
idkp->driver = &ide_gd_driver;
|
||||||
|
@ -373,6 +379,8 @@ static int ide_gd_probe(ide_drive_t *drive)
|
||||||
add_disk(g);
|
add_disk(g);
|
||||||
return 0;
|
return 0;
|
||||||
|
|
||||||
|
out_free_disk:
|
||||||
|
put_disk(g);
|
||||||
out_free_idkp:
|
out_free_idkp:
|
||||||
kfree(idkp);
|
kfree(idkp);
|
||||||
failed:
|
failed:
|
||||||
|
|
|
@ -17,7 +17,7 @@ struct ide_disk_obj {
|
||||||
ide_drive_t *drive;
|
ide_drive_t *drive;
|
||||||
struct ide_driver *driver;
|
struct ide_driver *driver;
|
||||||
struct gendisk *disk;
|
struct gendisk *disk;
|
||||||
struct kref kref;
|
struct device dev;
|
||||||
unsigned int openers; /* protected by BKL for now */
|
unsigned int openers; /* protected by BKL for now */
|
||||||
|
|
||||||
/* Last failed packet command */
|
/* Last failed packet command */
|
||||||
|
|
|
@ -169,7 +169,7 @@ typedef struct ide_tape_obj {
|
||||||
ide_drive_t *drive;
|
ide_drive_t *drive;
|
||||||
struct ide_driver *driver;
|
struct ide_driver *driver;
|
||||||
struct gendisk *disk;
|
struct gendisk *disk;
|
||||||
struct kref kref;
|
struct device dev;
|
||||||
|
|
||||||
/*
|
/*
|
||||||
* failed_pc points to the last failed packet command, or contains
|
* failed_pc points to the last failed packet command, or contains
|
||||||
|
@ -267,7 +267,7 @@ static DEFINE_MUTEX(idetape_ref_mutex);
|
||||||
|
|
||||||
static struct class *idetape_sysfs_class;
|
static struct class *idetape_sysfs_class;
|
||||||
|
|
||||||
static void ide_tape_release(struct kref *);
|
static void ide_tape_release(struct device *);
|
||||||
|
|
||||||
static struct ide_tape_obj *ide_tape_get(struct gendisk *disk)
|
static struct ide_tape_obj *ide_tape_get(struct gendisk *disk)
|
||||||
{
|
{
|
||||||
|
@ -279,7 +279,7 @@ static struct ide_tape_obj *ide_tape_get(struct gendisk *disk)
|
||||||
if (ide_device_get(tape->drive))
|
if (ide_device_get(tape->drive))
|
||||||
tape = NULL;
|
tape = NULL;
|
||||||
else
|
else
|
||||||
kref_get(&tape->kref);
|
get_device(&tape->dev);
|
||||||
}
|
}
|
||||||
mutex_unlock(&idetape_ref_mutex);
|
mutex_unlock(&idetape_ref_mutex);
|
||||||
return tape;
|
return tape;
|
||||||
|
@ -290,7 +290,7 @@ static void ide_tape_put(struct ide_tape_obj *tape)
|
||||||
ide_drive_t *drive = tape->drive;
|
ide_drive_t *drive = tape->drive;
|
||||||
|
|
||||||
mutex_lock(&idetape_ref_mutex);
|
mutex_lock(&idetape_ref_mutex);
|
||||||
kref_put(&tape->kref, ide_tape_release);
|
put_device(&tape->dev);
|
||||||
ide_device_put(drive);
|
ide_device_put(drive);
|
||||||
mutex_unlock(&idetape_ref_mutex);
|
mutex_unlock(&idetape_ref_mutex);
|
||||||
}
|
}
|
||||||
|
@ -308,7 +308,7 @@ static struct ide_tape_obj *ide_tape_chrdev_get(unsigned int i)
|
||||||
mutex_lock(&idetape_ref_mutex);
|
mutex_lock(&idetape_ref_mutex);
|
||||||
tape = idetape_devs[i];
|
tape = idetape_devs[i];
|
||||||
if (tape)
|
if (tape)
|
||||||
kref_get(&tape->kref);
|
get_device(&tape->dev);
|
||||||
mutex_unlock(&idetape_ref_mutex);
|
mutex_unlock(&idetape_ref_mutex);
|
||||||
return tape;
|
return tape;
|
||||||
}
|
}
|
||||||
|
@ -2256,15 +2256,17 @@ static void ide_tape_remove(ide_drive_t *drive)
|
||||||
idetape_tape_t *tape = drive->driver_data;
|
idetape_tape_t *tape = drive->driver_data;
|
||||||
|
|
||||||
ide_proc_unregister_driver(drive, tape->driver);
|
ide_proc_unregister_driver(drive, tape->driver);
|
||||||
|
device_del(&tape->dev);
|
||||||
ide_unregister_region(tape->disk);
|
ide_unregister_region(tape->disk);
|
||||||
|
|
||||||
ide_tape_put(tape);
|
mutex_lock(&idetape_ref_mutex);
|
||||||
|
put_device(&tape->dev);
|
||||||
|
mutex_unlock(&idetape_ref_mutex);
|
||||||
}
|
}
|
||||||
|
|
||||||
static void ide_tape_release(struct kref *kref)
|
static void ide_tape_release(struct device *dev)
|
||||||
{
|
{
|
||||||
struct ide_tape_obj *tape = to_ide_drv(kref, ide_tape_obj);
|
struct ide_tape_obj *tape = to_ide_drv(dev, ide_tape_obj);
|
||||||
ide_drive_t *drive = tape->drive;
|
ide_drive_t *drive = tape->drive;
|
||||||
struct gendisk *g = tape->disk;
|
struct gendisk *g = tape->disk;
|
||||||
|
|
||||||
|
@ -2407,7 +2409,12 @@ static int ide_tape_probe(ide_drive_t *drive)
|
||||||
|
|
||||||
ide_init_disk(g, drive);
|
ide_init_disk(g, drive);
|
||||||
|
|
||||||
kref_init(&tape->kref);
|
tape->dev.parent = &drive->gendev;
|
||||||
|
tape->dev.release = ide_tape_release;
|
||||||
|
dev_set_name(&tape->dev, dev_name(&drive->gendev));
|
||||||
|
|
||||||
|
if (device_register(&tape->dev))
|
||||||
|
goto out_free_disk;
|
||||||
|
|
||||||
tape->drive = drive;
|
tape->drive = drive;
|
||||||
tape->driver = &idetape_driver;
|
tape->driver = &idetape_driver;
|
||||||
|
@ -2436,6 +2443,8 @@ static int ide_tape_probe(ide_drive_t *drive)
|
||||||
|
|
||||||
return 0;
|
return 0;
|
||||||
|
|
||||||
|
out_free_disk:
|
||||||
|
put_disk(g);
|
||||||
out_free_tape:
|
out_free_tape:
|
||||||
kfree(tape);
|
kfree(tape);
|
||||||
failed:
|
failed:
|
||||||
|
|
|
@ -663,7 +663,7 @@ typedef struct ide_drive_s ide_drive_t;
|
||||||
#define to_ide_device(dev) container_of(dev, ide_drive_t, gendev)
|
#define to_ide_device(dev) container_of(dev, ide_drive_t, gendev)
|
||||||
|
|
||||||
#define to_ide_drv(obj, cont_type) \
|
#define to_ide_drv(obj, cont_type) \
|
||||||
container_of(obj, struct cont_type, kref)
|
container_of(obj, struct cont_type, dev)
|
||||||
|
|
||||||
#define ide_drv_g(disk, cont_type) \
|
#define ide_drv_g(disk, cont_type) \
|
||||||
container_of((disk)->private_data, struct cont_type, driver)
|
container_of((disk)->private_data, struct cont_type, driver)
|
||||||
|
|
Loading…
Add table
Reference in a new issue