[asterisk-bugs] [Asterisk 0018558]: [patch] p->chan can disappear between test and lock in deadlock avoidance in local_hangup

[Asterisk Bug Tracker]

A NOTE has been added to this issue. 
====================================================================== 
https://issues.asterisk.org/view.php?id=18558 
====================================================================== 
Reported By:                davidw
Assigned To:                
====================================================================== 
Project:                    Asterisk
Issue ID:                   18558
Category:                   Channels/chan_local
Reproducibility:            have not tried
Severity:                   minor
Priority:                   normal
Status:                     new
Asterisk Version:           SVN 
JIRA:                        
Regression:                 No 
Reviewboard Link:            
SVN Branch (only for SVN checkouts, not tarball releases): 1.6.2 
SVN Revision (number only!): 292867 
Request Review:              
====================================================================== 
Date Submitted:             2010-12-30 07:28 CST
Last Modified:              2011-01-04 13:02 CST
====================================================================== 
Summary:                    [patch] p->chan can disappear between test and lock
in deadlock avoidance in local_hangup
Description: 
r 259899 introduces an unlock/lock sequence on p-> chan in the deadlock
avoidance for a failed lock on p->owner, in local_hangup.  However, as
noted in a comment added/modified in r 292867, and assumed by code which
tests for NULL, p->chan may be nulled whilst the lock on p is off.

Depending on exactly where the NULL is set, as well as the case that is
handled, this could result in:

1) deferencing NULL, when calling lock;
2) applying a lock to a structure that is being destroyed.
====================================================================== 

---------------------------------------------------------------------- 
 (0130144) davidw (reporter) - 2011-01-04 13:02
 https://issues.asterisk.org/view.php?id=18558#c130144 
---------------------------------------------------------------------- 
I've uploaded a proposed patch.  I'd appreciate it if someone could check
it for obvious mistakes, as theoretical race conditions are difficult to
test.

Basically, it rearranges things, and uses try_lock, to ensure that
references from local_pvt are only used when it is locked.

Note, in reality, I actually coded this against a backport, but then
applied it against the latest 1.6.2 branch, to get correct line numbers. 
Any testing I do is likely to be against the back version.

Also, note that I have some reservations about the change to using
sched_yield that happened as the same time as the ao2_lock change.  As I
see it, it will work well with real time priorities, but with normal
priorities might result in going round the loop many times until ones CPU
factor rises enough to lose you the processor. 

Issue History 
Date Modified    Username       Field                    Change               
====================================================================== 
2011-01-04 13:02 davidw         Note Added: 0130144                          
======================================================================