[Home]

Summary:ASTERISK-13317: segmentation fault in local_queue_frame at chan_local.c:172
Reporter:sascha (sascha)Labels:
Date Opened:2009-01-07 10:16:25.000-0600Date Closed:2009-01-19 09:55:18.000-0600
Priority:CriticalRegression?No
Status:Closed/CompleteComponents:Channels/chan_local
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) 14189.patch
( 1) gdb.txt
( 2) gdb2.txt
Description:happened for the second time now today, but I have no idea what triggered it:

Failed to read a valid object file image from memory.
Core was generated by `/usr/sbin/asterisk -f -U asterisk -G asterisk -vvvg -c'.
Program terminated with signal 11, Segmentation fault.
#0  0x00002aaab2b1b5c6 in local_queue_frame (p=0x2aaab7c09eb0, isoutbound=1, f=0x84a418, us=0x2aaab7c011d0, us_locked=1)
   at chan_local.c:172
172             if (us && us->generator && other->generator)
(gdb) bt
#0  0x00002aaab2b1b5c6 in local_queue_frame (p=0x2aaab7c09eb0, isoutbound=1, f=0x84a418, us=0x2aaab7c011d0, us_locked=1)
   at chan_local.c:172
#1  0x00002aaab2b1bf87 in local_write (ast=0x2aaab7c011d0, f=0x84a418) at chan_local.c:324
#2  0x000000000043def8 in ast_write (chan=0x2aaab7c011d0, fr=<value optimized out>) at channel.c:2878
#3  0x00000000004619d9 in playtones_generator (chan=0x2aaab7c011d0, data=0x83ba20, len=320, samples=160) at indications.c:191
#4  0x000000000043c6e5 in generator_force (data=<value optimized out>) at channel.c:1623
ASTERISK-1  0x000000000043ff8e in __ast_read (chan=0x2aaab7c011d0, dropaudio=0) at channel.c:2104
ASTERISK-2  0x0000000000440d66 in ast_safe_sleep_conditional (chan=0x2aaab7c011d0, ms=20000, cond=0, data=0x0) at channel.c:2438
ASTERISK-3  0x00000000004779af in wait_for_hangup (chan=0x2aaab7c011d0, data=<value optimized out>) at pbx.c:5364
ASTERISK-4  0x0000000000477a5f in pbx_builtin_busy (chan=0x2aaab7c011d0, data=0x423766b0) at pbx.c:5403
ASTERISK-5  0x000000000048174b in pbx_extension_helper (c=0x2aaab7c011d0, con=<value optimized out>, context=0x2aaab7c01420 "macro-exten-vm",
   exten=0x2aaab7c01470 "s-BUSY", priority=4, label=<value optimized out>, callerid=0x2aaab7b16840 "03692350524", action=E_SPAWN)
   at pbx.c:537
ASTERISK-6 0x0000000000481b83 in ast_spawn_extension (c=0x2aaab7c09eb0, context=0x2aaab7c09f88 "?\021??*",
   exten=0x1 <Address 0x1 out of bounds>, priority=1, callerid=<value optimized out>) at pbx.c:2318
ASTERISK-7 0x00002aaab4a25b9a in _macro_exec (chan=0x2aaab7c011d0, data=<value optimized out>, exclusive=0) at app_macro.c:346
ASTERISK-8 0x000000000048174b in pbx_extension_helper (c=0x2aaab7c011d0, con=<value optimized out>, context=0x2aaab7c01420 "macro-exten-vm",
   exten=0x2aaab7c01470 "s-BUSY", priority=1, label=<value optimized out>, callerid=0x2aaab7b16840 "03692350524", action=E_SPAWN)
   at pbx.c:537
ASTERISK-9 0x0000000000483938 in __ast_pbx_run (c=0x2aaab7c011d0) at pbx.c:2318
ASTERISK-10 0x0000000000484639 in pbx_thread (data=0x2aaab7c09eb0) at pbx.c:2622
ASTERISK-11 0x00000000004aee6c in dummy_start (data=<value optimized out>) at utils.c:856
ASTERISK-12 0x00002aad0566ef1a in start_thread () from /lib/libpthread.so.0
ASTERISK-13 0x00002aad05c3b5d2 in clone () from /lib/libc.so.6
ASTERISK-14 0x0000000000000000 in ?? ()


in case it matters: it's on a debian linux, amd64, 2.6.24+13~etchnhalf.1 kernel. connected to the PSTN via mISDN 1.1.8  using a B410P.
Comments:By: Matthew Nicholson (mnicholson) 2009-01-07 14:30:21.000-0600

Please follow the instructions in doc/backtrace.txt for posting back traces.
By: Matthew Nicholson (mnicholson) 2009-01-07 15:00:14.000-0600

Looks like the code that causes this crash was introduced in rev 142927
By: Mark Michelson (mmichelson) 2009-01-07 17:33:35.000-0600

I actually fixed this for a local issue that was brought up, but I hadn't heard feedback on whether it had worked properly or not. I'll upload the same patch here and see if it helps.
By: Mark Michelson (mmichelson) 2009-01-07 17:37:07.000-0600

I've uploaded 14189.patch for testing. Please let us know if it works out for you.
By: sascha (sascha) 2009-01-08 04:46:21.000-0600

hmm, I may be blind, but the diff is a no-op, the new and old lines are identical!? and look the same like what I see in the chan_local.c from 1.4.23-rc3:

       /* do not queue frame if generator is on both local channels */
       if (us && us->generator && other->generator)
               return 0;

but thanks a lot for the quick response anyway!
By: sascha (sascha) 2009-01-08 04:56:15.000-0600

added gdb backtraces as requested.
By: Mark Michelson (mmichelson) 2009-01-08 11:09:28.000-0600

sascha, you're correct that the lines in the patch are identical, but the key is that they are moved to a point where we know that other will be non-NULL, which was not guaranteed where the lines were before.


By: Mark Michelson (mmichelson) 2009-01-08 11:15:15.000-0600

sascha, I cannot view the backtrace that you uploaded because Mantis is claiming that you have no license on file. Having a license is not required for anything that is not a code submission. Did you check the box that says "Check if this is a code or documentation submission."? If so, you need to re-upload the backtrace and be sure not to check that box.
By: sascha (sascha) 2009-01-08 14:56:13.000-0600

I think I didn't check the box, but just uploaded the same file again just in case.

Thanks for explaining the patch, I will give it a whirl tomorrow. Unfortunately, I don't know how to trigger the crash, so it will be hard to give a definitive report about it.


By: Mark Michelson (mmichelson) 2009-01-08 15:07:50.000-0600

Yes, it appears that the problem is actually a Mantis issue and not your fault. Sorry about that. Do you know if the backtrace is the same as what you originally posted in the description? If it is, then hopefully the patch I uploaded will fix the problem for you.
By: sascha (sascha) 2009-01-08 15:50:03.000-0600

it's the same, with more details included (bt full).
By: sascha (sascha) 2009-01-12 10:16:01.000-0600

ok, as I don't know what triggered the crashes, and as they happened rather random (it could even be days without crash), I can't give a definitive report.

having said this, with the patch applied, our asterisk so far survived two business days without crash, almost 4 days uptime in total now. at least I can tell that the patch doesn't seem to do any harm, everything is working fine.

I will yell if I see another crash, beside that I would recommend to add the patch to the code base.

thanks a lot for the quick help!
By: Leif Madsen (lmadsen) 2009-01-13 12:27:41.000-0600

I have marked this issue as Ready For Review! Hopefully it will get into the code soon enough. Thanks for the report!
By: Mark Michelson (mmichelson) 2009-01-13 13:34:57.000-0600

I'm going to leave this open for another day or two in case there is still a problem. I'm reasonably certain that the patch here will solve the problem, though.
By: Leif Madsen (lmadsen) 2009-01-14 12:55:55.000-0600

putnopvut: how invasive of a change is this?
By: Matthew Nicholson (mnicholson) 2009-01-14 13:10:36.000-0600

This patch looks good to me.  I just don't understand why local_queue_frame() gets called when p->chan and p->owner are null.  I don't know enough about chan_local and asterisk internals to understand that.  If that is normal, then this fix looks fine to me.  If anything, it should not cause any regressions that I can see.
By: Mark Michelson (mmichelson) 2009-01-14 13:25:15.000-0600

blitzrage: mnicholson put it well. It is not invasive at all.

I'm not 100% sure of all the details as to why one or both of p->chan or p->owner may be NULL, but there is a lot of weird stuff that goes on in chan_local. For instance, when a call is bridged using chan_local, the endpoint that p->chan was communicating with masquerades into the p->owner channel and p->chan is hung up. If there was a pending frame waiting to be queued onto p->chan, I suspect that could be a reason why it could be NULL.

The one thing that made me write the patch this way was that there already was logic in local_queue_frame to handle the case of p->chan being NULL, so I took that to mean that p->chan being NULL is not an error condition. The new code which checks for the presence of generators on both channels was added above this NULL check, so my patch to move it down below that seemed correct to me.
By: Leif Madsen (lmadsen) 2009-01-14 13:45:17.000-0600

Awesome thanks. That gives me the information I needed. What is your ETA on merging this in? Friday morning?
By: Mark Michelson (mmichelson) 2009-01-14 13:54:08.000-0600

Yeah, Friday sounds good. I have another bug open right now that I'm also waiting until Friday until merging, so I can knock them both out that day.
By: Mark Michelson (mmichelson) 2009-01-19 09:50:16.000-0600

Oops. I apparently forgot to commit this on Friday, I'll do it now.
By: Digium Subversion (svnbot) 2009-01-19 09:52:04.000-0600

Repository: asterisk
Revision: 169210

U   branches/1.4/channels/chan_local.c

------------------------------------------------------------------------
r169210 | mmichelson | 2009-01-19 09:52:03 -0600 (Mon, 19 Jan 2009) | 13 lines

Prevent a crash in chan_local due to a potential NULL pointer dereference

Move the check for if both channels on a local_pvt have generators to below
where p->chan is checked for NULLity (NULLness?). This prevents a crash from
occurring if p->chan is NULL.

(closes issue ASTERISK-13317)
Reported by: sascha
Patches:
     14189.patch uploaded by putnopvut (license 60)
Tested by: sascha


------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=169210
By: Digium Subversion (svnbot) 2009-01-19 09:53:54.000-0600

Repository: asterisk
Revision: 169211

_U  trunk/
U   trunk/channels/chan_local.c

------------------------------------------------------------------------
r169211 | mmichelson | 2009-01-19 09:53:54 -0600 (Mon, 19 Jan 2009) | 21 lines

Merged revisions 169210 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.4

........
r169210 | mmichelson | 2009-01-19 09:52:15 -0600 (Mon, 19 Jan 2009) | 13 lines

Prevent a crash in chan_local due to a potential NULL pointer dereference

Move the check for if both channels on a local_pvt have generators to below
where p->chan is checked for NULLity (NULLness?). This prevents a crash from
occurring if p->chan is NULL.

(closes issue ASTERISK-13317)
Reported by: sascha
Patches:
     14189.patch uploaded by putnopvut (license 60)
Tested by: sascha


........

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=169211
By: Digium Subversion (svnbot) 2009-01-19 09:54:49.000-0600

Repository: asterisk
Revision: 169212

_U  branches/1.6.0/
U   branches/1.6.0/channels/chan_local.c

------------------------------------------------------------------------
r169212 | mmichelson | 2009-01-19 09:54:49 -0600 (Mon, 19 Jan 2009) | 29 lines

Merged revisions 169211 via svnmerge from
https://origsvn.digium.com/svn/asterisk/trunk

................
r169211 | mmichelson | 2009-01-19 09:54:06 -0600 (Mon, 19 Jan 2009) | 21 lines

Merged revisions 169210 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.4

........
r169210 | mmichelson | 2009-01-19 09:52:15 -0600 (Mon, 19 Jan 2009) | 13 lines

Prevent a crash in chan_local due to a potential NULL pointer dereference

Move the check for if both channels on a local_pvt have generators to below
where p->chan is checked for NULLity (NULLness?). This prevents a crash from
occurring if p->chan is NULL.

(closes issue ASTERISK-13317)
Reported by: sascha
Patches:
     14189.patch uploaded by putnopvut (license 60)
Tested by: sascha


........

................

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=169212
By: Digium Subversion (svnbot) 2009-01-19 09:55:18.000-0600

Repository: asterisk
Revision: 169213

_U  branches/1.6.1/
U   branches/1.6.1/channels/chan_local.c

------------------------------------------------------------------------
r169213 | mmichelson | 2009-01-19 09:55:17 -0600 (Mon, 19 Jan 2009) | 29 lines

Merged revisions 169211 via svnmerge from
https://origsvn.digium.com/svn/asterisk/trunk

................
r169211 | mmichelson | 2009-01-19 09:54:06 -0600 (Mon, 19 Jan 2009) | 21 lines

Merged revisions 169210 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.4

........
r169210 | mmichelson | 2009-01-19 09:52:15 -0600 (Mon, 19 Jan 2009) | 13 lines

Prevent a crash in chan_local due to a potential NULL pointer dereference

Move the check for if both channels on a local_pvt have generators to below
where p->chan is checked for NULLity (NULLness?). This prevents a crash from
occurring if p->chan is NULL.

(closes issue ASTERISK-13317)
Reported by: sascha
Patches:
     14189.patch uploaded by putnopvut (license 60)
Tested by: sascha


........

................

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=169213