One per device just seems wasteful, when we already manintain a data structure to map minor numbers to devices, and we already have a PPS_MAX_SOURCES #define.
This is also a more comprehensive fix to the use-after-free bug that has already received a minimal patch. --- drivers/pps/pps.c | 66 ++++++++++++++++++++++++---------------------- include/linux/pps_kernel.h | 1 - 2 files changed, 34 insertions(+), 33 deletions(-) diff --git a/drivers/pps/pps.c b/drivers/pps/pps.c index 6437703..754b0b5 100644 --- a/drivers/pps/pps.c +++ b/drivers/pps/pps.c @@ -41,6 +41,8 @@ static dev_t pps_devt; static struct class *pps_class; +static struct cdev pps_cdev; + static DEFINE_MUTEX(pps_idr_lock); static DEFINE_IDR(pps_idr); @@ -244,17 +246,23 @@ static long pps_cdev_ioctl(struct file *file, static int pps_cdev_open(struct inode *inode, struct file *file) { - struct pps_device *pps = container_of(inode->i_cdev, - struct pps_device, cdev); - file->private_data = pps; - kobject_get(&pps->dev->kobj); - return 0; + int err = -ENXIO; + struct pps_device *pps; + + rcu_read_lock(); + pps = idr_find(&pps_idr, iminor(inode)); + if (pps) { + file->private_data = pps; + kobject_get(&pps->dev->kobj); + err = 0; + } + rcu_read_unlock(); + return err; } static int pps_cdev_release(struct inode *inode, struct file *file) { - struct pps_device *pps = container_of(inode->i_cdev, - struct pps_device, cdev); + struct pps_device *pps = file->private_data; kobject_put(&pps->dev->kobj); return 0; } @@ -277,8 +285,6 @@ static void pps_device_destruct(struct device *dev) { struct pps_device *pps = dev_get_drvdata(dev); - cdev_del(&pps->cdev); - /* Now we can release the ID for re-use */ pr_debug("deallocating pps%d\n", pps->id); mutex_lock(&pps_idr_lock); @@ -295,17 +301,14 @@ int pps_register_cdev(struct pps_device *pps) dev_t devt; mutex_lock(&pps_idr_lock); - /* Get new ID for the new PPS source */ - if (idr_pre_get(&pps_idr, GFP_KERNEL) == 0) { - mutex_unlock(&pps_idr_lock); - return -ENOMEM; - } - - /* Now really allocate the PPS source. + /* Get new ID for the new PPS source. * After idr_get_new() calling the new source will be freely available * into the kernel. */ - err = idr_get_new(&pps_idr, pps, &pps->id); + if (idr_pre_get(&pps_idr, GFP_KERNEL) == 0) + err = -ENOMEM; + else + err = idr_get_new(&pps_idr, pps, &pps->id); mutex_unlock(&pps_idr_lock); if (err < 0) @@ -321,33 +324,21 @@ int pps_register_cdev(struct pps_device *pps) devt = MKDEV(MAJOR(pps_devt), pps->id); - cdev_init(&pps->cdev, &pps_cdev_fops); - pps->cdev.owner = pps->info.owner; - - err = cdev_add(&pps->cdev, devt, 1); - if (err) { - pr_err("%s: failed to add char device %d:%d\n", - pps->info.name, MAJOR(pps_devt), pps->id); - goto free_idr; - } pps->dev = device_create(pps_class, pps->info.dev, devt, pps, "pps%d", pps->id); if (IS_ERR(pps->dev)) { err = PTR_ERR(pps->dev); - goto del_cdev; + goto free_idr; } /* Override the release function with our own */ pps->dev->release = pps_device_destruct; pr_debug("source %s got cdev (%d:%d)\n", pps->info.name, - MAJOR(pps_devt), pps->id); + MAJOR(devt), MINOR(devt)); return 0; -del_cdev: - cdev_del(&pps->cdev); - free_idr: mutex_lock(&pps_idr_lock); idr_remove(&pps_idr, pps->id); @@ -401,8 +392,9 @@ EXPORT_SYMBOL(pps_lookup_dev); static void __exit pps_exit(void) { - class_destroy(pps_class); + cdev_del(&pps_cdev); unregister_chrdev_region(pps_devt, PPS_MAX_SOURCES); + class_destroy(pps_class); } static int __init pps_init(void) @@ -422,12 +414,22 @@ static int __init pps_init(void) goto remove_class; } + cdev_init(&pps_cdev, &pps_cdev_fops); + pps_cdev.owner = THIS_MODULE; + err = cdev_add(&pps_cdev, pps_devt, PPS_MAX_SOURCES); + if (err < 0) { + pr_err("failed to register struct cdev\n"); + goto remove_region; + } + pr_info("LinuxPPS API ver. %d registered\n", PPS_API_VERS); pr_info("Software ver. %s - Copyright 2005-2007 Rodolfo Giometti " "<giome...@linux.it>\n", PPS_VERSION); return 0; +remove_region: + unregister_chrdev_region(pps_devt, PPS_MAX_SOURCES); remove_class: class_destroy(pps_class); diff --git a/include/linux/pps_kernel.h b/include/linux/pps_kernel.h index 7db3eb9..caca565 100644 --- a/include/linux/pps_kernel.h +++ b/include/linux/pps_kernel.h @@ -70,7 +70,6 @@ struct pps_device { unsigned int id; /* PPS source unique ID */ void const *lookup_cookie; /* pps_lookup_dev only */ - struct cdev cdev; struct device *dev; struct fasync_struct *async_queue; /* fasync method */ spinlock_t lock; -- 1.8.1.3 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/