[LinuxPPS] safe pps_register_source()

[Rodolfo Giometti]

On Thu, Aug 09, 2007 at 10:34:47AM -0400, Fabio Checconi wrote:

> > +
> > +	/* If we find a valid PPS source we lock it before leaving
> > +	 * the lock!
> > +	 */
> > +	if (!pps)
> > +		atomic_inc(&pps->usage);
> 
> here it seems you're dereferencing a null pointer (!pps)

Ach... fixed! :)

> >  	spin_unlock_irqrestore(&idr_lock, flags);
> >  
> >  	if (!pps)
> > @@ -251,5 +261,9 @@ void pps_event(int source, int event, void *data)
> >  	kill_fasync(&pps->async_queue, SIGIO, POLL_IN);
> >  
> >  	spin_unlock_irqrestore(&pps->lock, flags);
> > +
> > +	/* Now we can release the PPS source for (possible) deregistration */
> > +	atomic_dec(&pps->usage);
> > +	wake_up_all(&pps->usage_queue);
> 
> but a concurrent kfree(pps) on a different processor could be a problem,
> I have to think more about that...

Mmm... Maybe we can use the "idr_lock" since "kfree(pps)" is only
called inside a pps_unregister_source()...

Here my suggestion:

+   /* Now we can release the PPS source for (possible) deregistration */
+   spin_lock_irqsave(&idr_lock, flags);
+   atomic_dec(&pps->usage);
+   wake_up_all(&pps->usage_queue);
+   spin_unlock_irqrestore(&idr_lock, flags);

Ciao,

Rodolfo


-- 

GNU/Linux Solutions                  e-mail:    giometti at enneenne.com
Linux Device Driver                             giometti at gnudd.com
Embedded Systems                     		giometti at linux.it
UNIX programming                     phone:     +39 349 2432127