On Tue, Oct 23, 2007 at 02:08:19PM -0700, Roland Dreier wrote: > A few comments: > > > + dev_err(port->dev, "PPS support disabled due port \"%s\" is " > > + "in polling mode\n", > > I think "because" instead of "due" is closer to standard English.
Fixed. > > + printk(KERN_ERR "pps: %s: too much PPS sources in the system\n", > > + info->name); > > Similarly should be "many" instead of "much". Fixed. > > + /* Get new ID for the new PPS source */ > > + if (idr_pre_get(&pps_idr, GFP_KERNEL) == 0) { > > + err = -ENOMEM; > > + goto kfree_pps; > > + } > > + > > + spin_lock_irq(&idr_lock); > > + err = idr_get_new(&pps_idr, pps, &id); > > + spin_unlock_irq(&idr_lock); > > + > > + if (err < 0) > > + goto kfree_pps; > > You usually can handle idr_get_new() returning -EAGAIN by jumping back > to the idr_pre_get(), to handle someone else coming in and stealing > the memory you just preallocated. In this case it may not matter > since it's pretty unlikely that a lot of contexts are using the idr at > the same time. But anyway... I don't understand what you mean. Can you please submit an example code? > > +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. Mmm... you are right... can you please suggest to me how can I easily fix this problem? Ciao, 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/