Summary:	ASTERISK-11592: sipsock_read using unsafe structure
Reporter:	Norman Franke (norman)	Labels:
Date Opened:	2008-03-06 13:09:49.000-0600	Date Closed:	2008-06-27 17:36:39
Priority:	Minor	Regression?	No
Status:	Closed/Complete	Components:	Channels/chan_sip/General
Versions:		Frequency of Occurrence
Related Issues:
Environment:		Attachments:	( 0) 12162-lockfail.diff ( 1) sip.patch ( 2) sip2.patch
Description:	While trying to track down a series of crashes after ASTERISK-11504 was closed, I noticed a crash happened after I saw this on the console:    We could NOT get the channel lock for SIPstation032-06d12ea0! Running under valgrind, I noticed that sipsock_read was trying to lock p->owner (via ast_channel_trylock in the loop), and when this failed, it unlocks "p" then proceeds to use "p" and even p->owner (also unlocked and potentially free'd.) Valgrind noted, in the case of the crash, this section of code was accessing unallocated memory. I believe this happens rarely, when a channel has been closed at an inconvient time. After I applied this quick patch and the patch from ASTERISK-11391, I've never seen this problem while under valgrind after over a week of testing.
Comments:	By: Mark Michelson (mmichelson) 2008-03-07 10:19:02.000-0600 Could you re-upload the patch in unified diff format? Either use diff -u or svn diff to get the diff. It's very difficult to determine context with the type of diff currently uploaded. Thanks. By: Norman Franke (norman) 2008-03-07 14:15:27.000-0600 Rats. I meant to upload the unified patch, but copied the wrong one from my server. Sorry about that. Here  is the correct one. By: Joshua C. Colp (jcolp) 2008-05-05 09:16:32 qwell: In response to your poke on Friday re this yes I agree this will fix the issue in question and should be fine. By: Digium Subversion (svnbot) 2008-06-24 15:45:40 Repository: asterisk Revision: 124908 U   branches/1.4/channels/chan_sip.c ------------------------------------------------------------------------ r124908 | tilghman | 2008-06-24 15:45:37 -0500 (Tue, 24 Jun 2008) | 6 lines Don't access the pvt structure if unable to acquire the lock. (closes issue ASTERISK-11592) Reported by: norman Patches:       12162-lockfail.diff uploaded by qwell (license 4) ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=124908 By: Digium Subversion (svnbot) 2008-06-24 15:48:03 Repository: asterisk Revision: 124909 _U  trunk/ ------------------------------------------------------------------------ r124909 | tilghman | 2008-06-24 15:48:03 -0500 (Tue, 24 Jun 2008) | 13 lines Blocked revisions 124908 via svnmerge ........ r124908 | tilghman | 2008-06-24 15:52:43 -0500 (Tue, 24 Jun 2008) | 6 lines Don't access the pvt structure if unable to acquire the lock. (closes issue ASTERISK-11592) Reported by: norman Patches:       12162-lockfail.diff uploaded by qwell (license 4) ........ ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=124909 By: Digium Subversion (svnbot) 2008-06-27 17:36:39 Repository: asterisk Revision: 126112 _U  branches/1.6.0/ ------------------------------------------------------------------------ r126112 | tilghman | 2008-06-27 17:36:14 -0500 (Fri, 27 Jun 2008) | 286 lines Blocked revisions 114174,114298,115258,115518,117524,117812,118059,119077,120063,120372,120672,120732,121993,122315,122434,122616,122664,124744,124909,125894 via svnmerge ................ r114174 | qwell | 2008-04-16 12:31:02 -0500 (Wed, 16 Apr 2008) | 14 lines Blocked revisions 114173 via svnmerge ........ r114173 | qwell | 2008-04-16 12:30:09 -0500 (Wed, 16 Apr 2008) | 7 lines Fix "fallthrough" behavior here, so config options in a previously configured user don't override settings in general. (closes issue ASTERISK-11863) Reported by: tzafrir Patches:      chanzap_users_sections.diff uploaded by tzafrir (license 46) ........ ................ r114298 | tilghman | 2008-04-19 08:53:38 -0500 (Sat, 19 Apr 2008) | 11 lines Blocked revisions 114297 via svnmerge ........ r114297 | tilghman | 2008-04-19 08:49:50 -0500 (Sat, 19 Apr 2008) | 4 lines MOH usage information needs a terminating newline, or else "asterisk -rx 'help moh reload'" will hang.  Reported via -dev list, fixed by me. ........ ................ r115258 | bbryant | 2008-05-02 15:26:00 -0500 (Fri, 02 May 2008) | 9 lines Blocked revisions 115257 via svnmerge ........ r115257 | bbryant | 2008-05-02 15:25:42 -0500 (Fri, 02 May 2008) | 2 lines Add new "pri show version" command to show the libpri version for support reasons. ........ ................ r115518 | russell | 2008-05-07 13:17:43 -0500 (Wed, 07 May 2008) | 12 lines Blocked revisions 115517 via svnmerge ........ r115517 | russell | 2008-05-07 13:17:19 -0500 (Wed, 07 May 2008) | 5 lines Track peer references when stored in the sip_pvt struct as the peer related to a qualify ping or a subscription.  This fixes some realtime related crashes. (closes issue ASTERISK-11976) (closes issue ASTERISK-11945) ........ ................ r117524 | tilghman | 2008-05-21 13:45:26 -0500 (Wed, 21 May 2008) | 9 lines Blocked revisions 117523 via svnmerge ........ r117523 | tilghman | 2008-05-21 13:44:53 -0500 (Wed, 21 May 2008) | 2 lines Revert accidental commit of the last change ........ ................ r117812 | tilghman | 2008-05-22 11:50:32 -0500 (Thu, 22 May 2008) | 13 lines Blocked revisions 117809 via svnmerge ........ r117809 | tilghman | 2008-05-22 11:47:03 -0500 (Thu, 22 May 2008) | 6 lines Take into account the length of delimiters when calculating result string length. (closes issue ASTERISK-12059) Reported by: adomjan Patches:       func_realtime.c-longdelimiter.patch uploaded by adomjan (license 487) ........ ................ r118059 | tilghman | 2008-05-23 08:20:13 -0500 (Fri, 23 May 2008) | 9 lines Blocked revisions 118055 via svnmerge ........ r118055 | tilghman | 2008-05-23 08:18:44 -0500 (Fri, 23 May 2008) | 2 lines Add format type checking for recently de-inlined function ........ ................ r119077 | russell | 2008-05-29 15:49:48 -0500 (Thu, 29 May 2008) | 10 lines Blocked revisions 119076 via svnmerge ........ r119076 | russell | 2008-05-29 15:48:33 -0500 (Thu, 29 May 2008) | 3 lines Oddly enough, all of the contents of audiohook.h were in there twice.  I have removed the second copy. ........ ................ r120063 | tilghman | 2008-06-03 13:24:14 -0500 (Tue, 03 Jun 2008) | 15 lines Blocked revisions 120061 via svnmerge ........ r120061 | tilghman | 2008-06-03 13:23:32 -0500 (Tue, 03 Jun 2008) | 8 lines When listing the manager users, managers in users.conf are not shown, even though they are allowed to connect. (closes issue ASTERISK-11982) Reported by: bkruse Patches:       12594-managerusers-2.diff uploaded by qwell (license 4) Tested by: bkruse ........ ................ r120372 | russell | 2008-06-04 11:28:37 -0500 (Wed, 04 Jun 2008) | 11 lines Blocked revisions 120371 via svnmerge ........ r120371 | russell | 2008-06-04 11:26:43 -0500 (Wed, 04 Jun 2008) | 4 lines Make the "dialplan remove include" CLI command actually work.  Also, tweak some formatting, and make the success message a little bit more clear. (closes AST-52) ........ ................ r120672 | russell | 2008-06-05 11:39:25 -0500 (Thu, 05 Jun 2008) | 12 lines Blocked revisions 120671 via svnmerge ........ r120671 | russell | 2008-06-05 11:38:52 -0500 (Thu, 05 Jun 2008) | 5 lines It turns out that searching on the forwarding station isn't very useful for most people, so pull in the changes that allow searching for SMDI messages based on other components of the SMDI message.  Also, update the SMDI documentation. ........ ................ r120732 | russell | 2008-06-05 13:01:45 -0500 (Thu, 05 Jun 2008) | 9 lines Blocked revisions 120731 via svnmerge ........ r120731 | russell | 2008-06-05 13:01:25 -0500 (Thu, 05 Jun 2008) | 2 lines Add the UPGRADE.txt file from Asterisk 1.2, for handy reference. ........ ................ r121993 | twilson | 2008-06-11 18:48:38 -0500 (Wed, 11 Jun 2008) | 9 lines Blocked revisions 121992 via svnmerge ........ r121992 | twilson | 2008-06-11 18:47:23 -0500 (Wed, 11 Jun 2008) | 2 lines Backport fix for 11520--for some reason I didn't do this back in February when I patched for trunk. ........ ................ r122315 | jpeeler | 2008-06-12 14:11:23 -0500 (Thu, 12 Jun 2008) | 9 lines Blocked revisions 122314 via svnmerge ........ r122314 | jpeeler | 2008-06-12 14:08:20 -0500 (Thu, 12 Jun 2008) | 2 lines Adds DAHDI support alongside Zaptel. DAHDI usage favored, but all Zap stuff should continue working. Release announcement to follow. ........ ................ r122434 | jpeeler | 2008-06-12 18:09:16 -0500 (Thu, 12 Jun 2008) | 12 lines Blocked revisions 122208 via svnmerge ........ r122208 | jpeeler | 2008-06-12 10:46:08 -0500 (Thu, 12 Jun 2008) | 5 lines (closes issue ASTERISK-11622) Reported by: davidw Patch by: Corydon76, modified by me to work properly with ParkAndAnnounce app ........ ................ r122616 | jpeeler | 2008-06-13 12:38:28 -0500 (Fri, 13 Jun 2008) | 13 lines Blocked revisions 122613 via svnmerge ........ r122613 | jpeeler | 2008-06-13 12:36:56 -0500 (Fri, 13 Jun 2008) | 6 lines (closes issue ASTERISK-12184) Reported by: Netview Tested by: jpeeler Use correct location to search for tonezone. ........ ................ r122664 | jpeeler | 2008-06-13 13:58:29 -0500 (Fri, 13 Jun 2008) | 8 lines Blocked revisions 122663 via svnmerge ........ r122663 | jpeeler | 2008-06-13 13:57:24 -0500 (Fri, 13 Jun 2008) | 1 line fixed dahdi compatability header from assuming either dahdi or zaptel is installed (may not have either) ........ ................ r124744 | kpfleming | 2008-06-23 16:24:34 -0500 (Mon, 23 Jun 2008) | 10 lines Blocked revisions 124743 via svnmerge ........ r124743 | kpfleming | 2008-06-23 16:22:08 -0500 (Mon, 23 Jun 2008) | 3 lines emit a warning if the old IAX2 call searching code finds a call when the new code did not... so that we can get rid of the old code in 2-3 months ........ ................ r124909 | tilghman | 2008-06-24 15:55:06 -0500 (Tue, 24 Jun 2008) | 13 lines Blocked revisions 124908 via svnmerge ........ r124908 | tilghman | 2008-06-24 15:52:43 -0500 (Tue, 24 Jun 2008) | 6 lines Don't access the pvt structure if unable to acquire the lock. (closes issue ASTERISK-11592) Reported by: norman Patches:       12162-lockfail.diff uploaded by qwell (license 4) ........ ................ r125894 | tilghman | 2008-06-27 11:48:05 -0500 (Fri, 27 Jun 2008) | 14 lines Blocked revisions 125893 via svnmerge ........ r125893 | tilghman | 2008-06-27 11:46:05 -0500 (Fri, 27 Jun 2008) | 7 lines Since HAVE_DAHDI is defined to HAVE_ZAPTEL in dahdi_compat.h, we must first check for HAVE_ZAPTEL. (closes issue ASTERISK-12259) Reported by: opticron Patches:       tonezone_compat.diff uploaded by opticron (license 267) ........ ................ ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=126112