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: [email protected]
Linux Device Driver [email protected]
Embedded Systems [email protected]
UNIX programming phone: +39 349 2432127
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
