ASTERISK-12311: [patch] segfault in app

[Home]

Summary: ASTERISK-12311: [patch] segfault in app_chanspy.cpp

Reporter: Andrei Tanas (andrew53) Labels:

Date Opened: 2008-07-03 14:37:05 Date Closed: 2008-07-03 16:05:48

Priority: Critical Regression? No

Status: Closed/Complete Components: Applications/app_chanspy

Versions: Frequency of
Occurrence

Related
Issues:

Environment: Attachments: ( 0) chanspy_nullref.patch

Description: Due to some kind of race condition that I wasn't able to identify exactly (most likely a channel disconnected while trying to attach to it for monitoring), ast_bridged_channel(spyee) (called from channel_spy) returns null pointer which start_spying tries to dereference without checking. Backtrace and patch are attached.

****** ADDITIONAL INFORMATION ******

0x00007fd8991d6435 in start_spying (chan=0x0, spychan_name=0x40958ac0 "SIP/workpc-9423d0a8", audiohook=0x4095cd40) at app_chanspy.c:286
286 ast_log(LOG_NOTICE, "Attaching %s to %s\n", spychan_name, chan->name);
(gdb) bt
#0 0x00007fd8991d6435 in start_spying (chan=0x0, spychan_name=0x40958ac0 "SIP/workpc-9423d0a8", audiohook=0x4095cd40) at app_chanspy.c:286
#1 0x00007fd8991d6b36 in channel_spy (chan=0x7fd8942409c0, spyee_chanspy_ds=0x4095f820, volfactor=0x4095eedc, fd=0, flags=0x409607c0,
exitcontext=0x40960620 "") at app_chanspy.c:369
#2 0x00007fd8991d8bcf in common_exec (chan=0x7fd8942409c0, flags=0x409607c0, volfactor=0, fd=0, mygroup=0x0, myenforced=0x0,
spec=0x409606d0 "SIP/spa3102a", exten=0x0, context=0x0, mailbox=0x0, name_context=0x0) at app_chanspy.c:844
#3 0x00007fd8991d9a38 in chanspy_exec (chan=0x7fd8942409c0, data=0x409606d0) at app_chanspy.c:975
#4 0x00000000004d3f27 in pbx_exec (c=0x7fd8942409c0, app=0x7fd89402c870, data=0x40963d60) at pbx.c:747
ASTERISK-1 0x00000000004dcc03 in pbx_extension_helper (c=0x7fd8942409c0, con=0x0, context=0x7fd89424190a "local_phone",
exten=0x7fd89424195a "*999SIP/spa3102a", priority=5, label=0x0, callerid=0x7fd89422a890 "workpc", action=E_SPAWN, found=0x40965f1c,
combined_find_spawn=1) at pbx.c:2986
ASTERISK-2 0x00000000004de28c in ast_spawn_extension (c=0x7fd8942409c0, context=0x7fd89424190a "local_phone", exten=0x7fd89424195a "*999SIP/spa3102a",
priority=5, callerid=0x7fd89422a890 "workpc", found=0x40965f1c, combined_find_spawn=1) at pbx.c:3420
ASTERISK-3 0x00000000004deb53 in __ast_pbx_run (c=0x7fd8942409c0) at pbx.c:3521
ASTERISK-4 0x00000000004e02d3 in pbx_thread (data=0x7fd8942409c0) at pbx.c:3801
ASTERISK-5 0x000000000053f303 in dummy_start (data=0x7fd89422a210) at utils.c:1024
ASTERISK-6 0x00007fd89bbfc3f7 in start_thread () from /lib/libpthread.so.0
ASTERISK-7 0x00007fd89c0edb2d in clone () from /lib/libc.so.6
ASTERISK-8 0x0000000000000000 in ?? ()

Comments: By: Mark Michelson (mmichelson) 2008-07-03 15:03:47

Ah, I see what's happening here. Apparently, Chanspy() is being used on a channel which is not bridged. This wasn't taken into consideration when adding barge support to trunk. The patch you provided helps for the case where start_spying is called with a NULL channel as an argument, but the problem is that the channel could become NULL during that function call. It is important to have the bridged channel locked (if it's non-NULL) through the entire start_spying call. I will have a fix for this committed very shortly.
By: Digium Subversion (svnbot) 2008-07-03 15:11:55

Repository: asterisk
Revision: 127831

U trunk/apps/app_chanspy.c

------------------------------------------------------------------------
r127831 | mmichelson | 2008-07-03 15:11:52 -0500 (Thu, 03 Jul 2008) | 6 lines

Fix a crash when attempting to spy on an unbridged channel.

(closes issue ASTERISK-12311)
Reported by: andrew53

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

http://svn.digium.com/view/asterisk?view=rev&revision=127831
By: Andrei Tanas (andrew53) 2008-07-03 15:43:37

"The patch you provided helps for the case where start_spying is called with a NULL channel as an argument"
What about it then? Or are you saying that locking the channel will also guarantee that start_spying never gets called when the channel is NULL?
By: Andrei Tanas (andrew53) 2008-07-03 15:44:46

Sorry, stupid of me, of course it's there, never mind.
By: Andrei Tanas (andrew53) 2008-07-03 15:48:29

Why not like this though?

Index: apps/app_chanspy.c
===================================================================
--- apps/app_chanspy.c (revision 127831)
+++ apps/app_chanspy.c (working copy)
@@ -364,7 +364,7 @@
start_spying(spyee, spyer_name, &csth.whisper_audiohook); /* Unlocks spyee */
if ((spyee_bridge = ast_bridged_channel(spyee))) {
ast_channel_lock(spyee_bridge);
- start_spying(ast_bridged_channel(spyee), spyer_name, &csth.bridge_whisper_audiohook);
+ start_spying(spyee_bridge, spyer_name, &csth.bridge_whisper_audiohook);
ast_channel_unlock(spyee_bridge);
}
ast_channel_unlock(spyee);
By: Mark Michelson (mmichelson) 2008-07-03 15:51:34

Good call. I'll make that change.
By: Andrei Tanas (andrew53) 2008-07-03 16:00:46

spyee_bridge declaration is missing

Index: apps/app_chanspy.c
===================================================================
--- apps/app_chanspy.c (revision 127831)
+++ apps/app_chanspy.c (working copy)
@@ -323,6 +323,7 @@
struct ast_frame *f;
struct ast_silence_generator *silgen = NULL;
struct ast_channel *spyee = NULL;
+ struct ast_channel *spyee_bridge = NULL;
const char *spyer_name;

ast_channel_lock(chan);
@@ -364,7 +365,7 @@
start_spying(spyee, spyer_name, &csth.whisper_audiohook); /* Unlocks spyee */
if ((spyee_bridge = ast_bridged_channel(spyee))) {
ast_channel_lock(spyee_bridge);
- start_spying(ast_bridged_channel(spyee), spyer_name, &csth.bridge_whisper_audiohook);
+ start_spying(spyee_bridge, spyer_name, &csth.bridge_whisper_audiohook);
ast_channel_unlock(spyee_bridge);
}
ast_channel_unlock(spyee);
By: Mark Michelson (mmichelson) 2008-07-03 16:05:19

You need to svn update. I forgot to add it originally but I fixed that real quick after the buildbot told me that I broke the build :)