Messages in this thread | | | Date | Thu, 25 Oct 2007 10:39:16 +0200 | From | Rodolfo Giometti <> | Subject | Re: [PATCH] LinuxPPS - PPS support for Linux |
| |
On Tue, Oct 23, 2007 at 02:08:19PM -0700, Roland Dreier wrote:
> > +void pps_unregister_source(int source) > > ... > > + wait_event(pps->usage_queue, atomic_read(&pps->usage) == 0); > > + > > + pps_sysfs_remove_source_entry(pps); > > + pps_unregister_cdev(pps); > > + kfree(pps); > > This reference counting looks dubious to me... later on in the code > you have: > > > +static int pps_cdev_open(struct inode *inode, struct file *file) > > +{ > > + struct pps_device *pps = container_of(inode->i_cdev, > > + struct pps_device, cdev); > > + > > + /* Lock the PPS source against (possible) deregistration */ > > + atomic_inc(&pps->usage); > > with no locking, so I see no reason why the atomic_inc() couldn't > happen right after the wait_event() sees a count of 0 and lets the > deregistration continue. Which would lead to use-after-free.
Since pps_unregister_source() is defined as:
void pps_unregister_source(int source) { struct pps_device *pps;
spin_lock_irq(&idr_lock); pps = idr_find(&pps_idr, source);
if (!pps) { spin_unlock_irq(&idr_lock); return; }
/* This should be done first in order to deny IRQ handlers * to access PPS structs */
idr_remove(&pps_idr, pps->id); spin_unlock_irq(&idr_lock);
wait_event(pps->usage_queue, atomic_read(&pps->usage) == 0);
pps_sysfs_remove_source_entry(pps); pps_unregister_cdev(pps); kfree(pps); }
changing pps_cdev_open() as follow should resolve this problem:
static int pps_cdev_open(struct inode *inode, struct file *file) { struct pps_device *pps = container_of(inode->i_cdev, struct pps_device, cdev); int found;
spin_lock_irq(&idr_lock); found = idr_find(&pps_idr, pps->id) != NULL;
/* Lock the PPS source against (possible) deregistration */ if (found) atomic_inc(&pps->usage);
spin_unlock_irq(&idr_lock);
if (!found) return -ENODEV;
file->private_data = pps;
return 0; }
Is that right?
Rodolfo
--
GNU/Linux Solutions e-mail: giometti@enneenne.com Linux Device Driver giometti@gnudd.com Embedded Systems giometti@linux.it UNIX programming phone: +39 349 2432127 - 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/
| |