[asterisk-bugs] [Zaptel 0010806]: Channel opening and unregistering race

[noreply at bugs.digium.com]

A NOTE has been added to this issue. 
====================================================================== 
http://bugs.digium.com/view.php?id=10806 
====================================================================== 
Reported By:                Matti
Assigned To:                
====================================================================== 
Project:                    Zaptel
Issue ID:                   10806
Category:                   zaptel (the module)
Reproducibility:            sometimes
Severity:                   crash
Priority:                   normal
Status:                     new
Zaptel Version:              SVN 
SVN Branch (only for SVN checkouts, not tarball releases):  1.4  
SVN Revision (number only!): 3052 
Disclaimer on File?:        N/A 
Request Review:              
====================================================================== 
Date Submitted:             09-23-2007 03:45 CDT
Last Modified:              10-22-2007 13:22 CDT
====================================================================== 
Summary:                    Channel opening and unregistering race
Description: 
Function zt_specchan_open can have a race condition with zt_chan_unreg
resulting in crashing of the program (Asterisk) or the whole computer
during the opening of a Zap device file.
====================================================================== 

---------------------------------------------------------------------- 
 meneault - 10-22-07 13:22  
---------------------------------------------------------------------- 
I did take a look at your patch (the last one).
I do agree on the fact that spans must be locked before any access.

But I do not agree on the fact we should use semaphore, because 'chans'
and 'spans' may be accessed from interrupt context (not simply user
context).
One of the example is zt_alarm_notify (you did notice it because you
carefully used down_read_try_lock).
What do you do when you cannot take the lock? nothing... For
zt_alarm_notify it may not hurt much but this won't be the only case.

At the moment zaptel drivers are full of race conditions (mainly because
of unlocked accesses) and you should have noticed that there are some
'chans' unlocked accesses that could be protected in the future.
One example is in __zt_process_putaudio_chunk (in interrupt context).

So we are obliged to use spinlocks.


For the 'spans' locking I recommend the use of another rw_lock spinlock
and a clear locking policy (i.e if you've got chans_lock you cannot take
spans_lock
but the opposite is allowed).

The drawback of using spinlock is that we must be sure not to sleep while
holding the lock, I didn't check all zaptel code to see if it may happen or
not.

About your crash problem, I don't really understand it. Is it:
- a deadlock (e.g. spinlock held while sleeping)
- an unprotected access that would lead to segfault/exit (e.g.
inconsistent value)

Getting zaptel-base out of race conditions is a great thing, It wouldn't
be so hard to get it right (However this would lead to a patch of about
500kB :)). 

Issue History 
Date Modified   Username       Field                    Change               
====================================================================== 
10-22-07 13:22  meneault       Note Added: 0072375                          
======================================================================