Summary:	ASTERISK-09957: [patch] channel_find_locked causes loops (e.g. in handle_chanlist)
Reporter:	Marcus Hunger (fnordian)	Labels:
Date Opened:	2007-07-25 09:44:58	Date Closed:	2007-07-30 13:39:09
Priority:	Critical	Regression?	No
Status:	Closed/Complete	Components:	Core/Channels
Versions:		Frequency of Occurrence
Related Issues:
Environment:		Attachments:	( 0) asterisk-1.4.9-channel.c.patch
Description:	Hi, an error in channel_find_locked causes asterisk to loop under certain circumstances. This happens when the function retries to find a channel because it was locked. The prev-reference is already overwritten (NULL) when the function walks through the list again. So the first element matches and the whole thing (i.e. the using function) might start to loop. ****** ADDITIONAL INFORMATION ****** This is related to http://bugs.digium.com/view.php?id=10055.
Comments:	By: Joshua C. Colp (jcolp) 2007-07-30 09:14:18 I've looked over the existing code, the patch, and don't exactly understand what you mean. Can you clarify a bit more? By: Marcus Hunger (fnordian) 2007-07-30 10:24:20 channel_find_locked loops through the channel list and tries to find the next matching channel. if there's a previous channel (prev), it loops through the list, until it finds prev, skips it and sets prev = NULL. Then the function tries to lock the channel and if that fails, it starts again searching the channel list _from_the_beginning_ because prev is set to zero. So the loop stops at the first element matching, instead of walking through the list until prev. The patch restores prev before the funtion does the list-traversal again and so fixes the bug. A better approach might be to only retry the locking instead of list-traversal but this would block the list and might lead to a deadlock. By: Digium Subversion (svnbot) 2007-07-30 10:30:32 Repository: asterisk Revision: 77771 ------------------------------------------------------------------------ r77771 | file | 2007-07-30 10:30:31 -0500 (Mon, 30 Jul 2007) | 6 lines (closes issue ASTERISK-9957) Reported by: fnordian Patches:      asterisk-1.4.9-channel.c.patch uploaded by fnordian (license 110) Restore previous behavior where if we failed to lock the channel we wanted we would return to exactly the same point as if we had just reentered the function. ------------------------------------------------------------------------ By: Digium Subversion (svnbot) 2007-07-30 10:32:10 Repository: asterisk Revision: 77772 ------------------------------------------------------------------------ r77772 | file | 2007-07-30 10:32:09 -0500 (Mon, 30 Jul 2007) | 14 lines Merged revisions 77771 via svnmerge from https://origsvn.digium.com/svn/asterisk/branches/1.4 ........ r77771 | file | 2007-07-30 12:47:52 -0300 (Mon, 30 Jul 2007) | 6 lines (closes issue ASTERISK-9957) Reported by: fnordian Patches:      asterisk-1.4.9-channel.c.patch uploaded by fnordian (license 110) Restore previous behavior where if we failed to lock the channel we wanted we would return to exactly the same point as if we had just reentered the function. ........ ------------------------------------------------------------------------ By: Digium Subversion (svnbot) 2007-07-30 12:12:22 Repository: asterisk Revision: 77780 ------------------------------------------------------------------------ r77780 | russell | 2007-07-30 12:12:22 -0500 (Mon, 30 Jul 2007) | 16 lines (closes issue ASTERISK-9957) Reported by: fnordian Patches:      asterisk-1.4.9-channel.c.patch uploaded by fnordian (license 110)      Additional changes by me Fix some problems in channel_find_locked() which can cause an infinite loop. The reference to the previous channel is set to NULL in some cases.  These changes ensure that the reference to the previous channel gets restored before needing it again. I'm not convinced that the code that is setting it to NULL is really the right thing to do.  However, I am making these changes to fix the obvious problem and just leaving an XXX comment that it needs a better explanation that what is there now. ------------------------------------------------------------------------ By: Digium Subversion (svnbot) 2007-07-30 12:14:10 Repository: asterisk Revision: 77781 ------------------------------------------------------------------------ r77781 | russell | 2007-07-30 12:14:10 -0500 (Mon, 30 Jul 2007) | 24 lines Merged revisions 77780 via svnmerge from https://origsvn.digium.com/svn/asterisk/branches/1.4 ........ r77780 | russell | 2007-07-30 12:29:43 -0500 (Mon, 30 Jul 2007) | 16 lines (closes issue ASTERISK-9957) Reported by: fnordian Patches:      asterisk-1.4.9-channel.c.patch uploaded by fnordian (license 110)      Additional changes by me Fix some problems in channel_find_locked() which can cause an infinite loop. The reference to the previous channel is set to NULL in some cases.  These changes ensure that the reference to the previous channel gets restored before needing it again. I'm not convinced that the code that is setting it to NULL is really the right thing to do.  However, I am making these changes to fix the obvious problem and just leaving an XXX comment that it needs a better explanation that what is there now. ........ ------------------------------------------------------------------------ By: Digium Subversion (svnbot) 2007-07-30 13:37:54 Repository: asterisk Revision: 77785 ------------------------------------------------------------------------ r77785 | russell | 2007-07-30 13:37:54 -0500 (Mon, 30 Jul 2007) | 3 lines file and I both committed changes for issue ASTERISK-9957.  Remove a duplicated assignment to restore the original value of the previous channel. ------------------------------------------------------------------------ By: Digium Subversion (svnbot) 2007-07-30 13:39:09 Repository: asterisk Revision: 77786 ------------------------------------------------------------------------ r77786 | russell | 2007-07-30 13:39:08 -0500 (Mon, 30 Jul 2007) | 11 lines Merged revisions 77785 via svnmerge from https://origsvn.digium.com/svn/asterisk/branches/1.4 ........ r77785 | russell | 2007-07-30 13:55:15 -0500 (Mon, 30 Jul 2007) | 3 lines file and I both committed changes for issue ASTERISK-9957.  Remove a duplicated assignment to restore the original value of the previous channel. ........ ------------------------------------------------------------------------