[asterisk-dev] [Code Review] 2983: app_queue: crash if first agent is "busy"

[Kevin Harwell]

> On Oct. 31, 2013, 4:59 p.m., Mark Michelson wrote:
> > branches/12/apps/app_queue.c, lines 4470-4471
> > <https://reviewboard.asterisk.org/r/2983/diff/1/?file=47841#file47841line4470>
> >
> >     Is this actually necessary? Looking at how call_next is used, it appears that prev->call_next should already be NULL if the if case that this else is paired with is not entered.

Yes.  This is essentially what caused the problem.  prev->call_next can potentially point to a "callattempt" object that contains an empty channel/not "stillgoing".  The intention of the code is to skip these "nulled" channel objects while building the list.  It does this correctly except in the case of subsequent list builds and when the last item checked is not still going.


- Kevin


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/2983/#review10084
-----------------------------------------------------------


On Oct. 31, 2013, 4:14 p.m., Kevin Harwell wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/2983/
> -----------------------------------------------------------
> 
> (Updated Oct. 31, 2013, 4:14 p.m.)
> 
> 
> Review request for Asterisk Developers, Matt Jordan and Mark Michelson.
> 
> 
> Bugs: ASTERISK-22644
>     https://issues.asterisk.org/jira/browse/ASTERISK-22644
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> If the first agent/member (via CLI "queue show") in a queue is "busy" (dnd, circuit busy, etc...) and no agents answered then app_queue would crash.  This occurred because while the calling of agent(s) remained valid the channel on "busy" agent would be set to NULL and then later dereferenced upon a second "rna" function call.  The original intention of the code is to have only valid "call attempt" objects (channels != NULL) checked while attempting to call agent(s).  It does this by building a "call_next" list of valid "call attempt" objects.  In the case of the "busy" agent subsequent builds of the valid "call attempt" list would sometimes include (the case mentioned above) an invalid "call attempt" object.
> 
> The fix was to make sure the "call attempt" list was appropriately built on every iteration.  A NULL sanity check was also added at the original offending spot of the crash just in case another one slipped by somehow.
> 
> 
> Diffs
> -----
> 
>   branches/12/apps/app_queue.c 402326 
> 
> Diff: https://reviewboard.asterisk.org/r/2983/diff/
> 
> 
> Testing
> -------
> 
> Recreated the crash by creating a queue with 3 members with the first member's status set to "dnd".  Once the fix was in ran the test again to and made sure Asterisk no longer crashed.  After that I tried various combinations of phones being set to "busy" at various times during agent calling, including none and all three.
> 
> 
> Thanks,
> 
> Kevin Harwell
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20131101/dda46551/attachment.html>