ASTERISK-11777: Asterisk crashes everytime i try to dial a realtime peer. e.g.DIAL(SIP/peer/number,60,tTwW)

[Home]

Summary: ASTERISK-11777: Asterisk crashes everytime i try to dial a realtime peer. e.g.DIAL(SIP/peer/number,60,tTwW)

Reporter: Vadim Sherbakov (vinsik) Labels:

Date Opened: 2008-04-03 07:36:11 Date Closed: 2008-04-07 16:31:58

Priority: Critical Regression? No

Status: Closed/Complete Components: Channels/chan_sip/General

Versions: Frequency of
Occurrence

Related
Issues:

Environment: Attachments: ( 0) 12362.patch
( 1) 12362-opposite.patch
( 2) core_backtrace_1_4_19.txt

Description: Asterisk crashes everytime i try to dial a realtime peer.

Command line output:
-- AGI Script Executing Application: (SET) Options: (CDR(accountcode)=10000)
-- AGI Script Executing Application: (SET) Options: (CALLERID(num)=123456)
-- AGI Script Executing Application: (DIAL) Options: (SIP/roxser/123456|60|tTwW)
Disconnected from Asterisk server

Local realtime users work fine. Also all local CMD's work.
Cannot find an logical reason. Reverting back to 1.4.18, as it works ok.

Attached is GDB of asterisk process.
Asterisk debug does not reveal anything relevant.

Comments: By: snuffy (snuffy) 2008-04-03 08:20:27

I noticed 'optimised out' in your backtrace.
could you replicate with the 'dont compile' flag checked and post the backtrace
By: Mark Michelson (mmichelson) 2008-04-03 08:57:55

The problem appeared to stem from a memcmp that involved a NULL sockaddr_in pointer. I have uploaded a potential patch for this. The logic seems right by me, but I'd appreciate it if you'd give it a quick test to be sure that it works as I expect it to.

As snuffy noted, for future backtraces, if you could upload unoptimized backtraces it is generally more helpful. Although, instead of "dont compile' I believe he meant to say to select the DONT_OPTIMIZE flag in the "compile options" section of menuselect.

Thanks for reporting this!
By: Vadim Sherbakov (vinsik) 2008-04-03 10:54:47

snuffy, will do. :)
putnopvut, it worked fine. Thanks for fast reply.

I am still wondering though, why cant i dial like this?
SIP/peernamefromsql/number <-

Is this method obsolete?

res_config_mysql modules checks the host like this:
[Apr 3 18:49:45] DEBUG[9061] res_config_mysql.c: MySQL RealTime: Retrieve SQL: SELECT * FROM server_sip_users WHERE name = 'roxser' AND host = 'dynamic'
And since it has host='HOSTNAME', asterisk cannot find peer properly.
Only way for me to work around, is to use a hostname:

SIP/peerhostname/number

But with just the hostname i cannot define username/passwords to
DIAL CMD if the other side requests them. This modification in the asterisk code has been made for a reason, i presume. So what am i doing wrong?

I dont know did i make any sence, but bare with me. :)
By: Vadim Sherbakov (vinsik) 2008-04-03 11:10:44

Basicly i was thinking to add some code that checks SQL like so:
res_config_mysql.c: MySQL RealTime: Retrieve SQL: SELECT * FROM server_sip_users WHERE name = 'peername' AND type = 'peer' first. Then the other way.
By: Vadim Sherbakov (vinsik) 2008-04-03 11:27:12

i think the problem is here:

/* 1.4.18 - worked */
if (!strcasecmp(var->name, "host")) {
struct in_addr sin2;
struct ast_dnsmgr_entry *dnsmgr = NULL;
memset(&sin2, 0, sizeof(sin2));
if ((ast_dnsmgr_lookup(tmp->value, &sin2, &dnsmgr) < 0) || (memcmp(&sin2, &sin->sin_addr, sizeof(sin2)) != 0)) {

/* 1.4.19 - seg.faults when sin=0x0 */
if (!strcasecmp(tmp->name, "host")) {
struct hostent *hp;
struct ast_hostent ahp;
if (!(hp = ast_gethostbyname(tmp->value, &ahp)) || (memcmp(&hp->h_addr, &sin->sin_addr, sizeof(hp->h_addr)))) {
By: Mark Michelson (mmichelson) 2008-04-03 14:16:25

vinsik

Yes, that is exactly the problem which my patch attempts to fix. We don't want to crash if sin is NULL.

Regarding the other problem you posted (since you use an explicit 'hostname' field for your sip peers), there is a comment in chan_sip which explains what it is attempting to do. Here's the comment:

/*!\note
* If this one loaded something, then we need to ensure that the host
* field matched. The only reason why we can't have this as a criteria
* is because we only have the IP address and the host field might be
* set as a name (and the reverse PTR might not match).
*/

I interpreted this to mean that we have an IP address, and so we need to look up the hostname to make sure its A record matches the IP address we have. If we don't have an IP address (in other words, sin is NULL), then we can't make a match. Perhaps this should be interpreted the opposite way. If we have no IP address we're expecting, then match the peer based solely on its name and don't worry about the hostname. As an experiment, I'll upload a patch that exhibits this behavior...
By: Mark Michelson (mmichelson) 2008-04-03 14:17:59

I have uploaded 12362-opposite.patch, which exhibits the behavior I mentioned at the end of my last note. See if this will allow you to dial the way you were talking about before.
By: german aracil boned (tecnoxarxa) 2008-04-06 08:38:55

I have this problem and I reported this:
http://bugs.digium.com/view.php?id=12372

My propose for solution is change only one line:

Old:

if (!strcasecmp(tmp->name, "host")) {
^^^

New:

if (!strcasecmp(var->name, "host")) {
^^^

I have testing now changes and this work good. Any problem for this ?
By: Digium Subversion (svnbot) 2008-04-07 10:11:55

Repository: asterisk
Revision: 113012

U branches/1.4/channels/chan_sip.c

------------------------------------------------------------------------
r113012 | jpeeler | 2008-04-07 10:11:51 -0500 (Mon, 07 Apr 2008) | 7 lines

(closes issue ASTERISK-11777)
(closes issue ASTERISK-11787)
Reported by: vinsik
Tested by: tecnoxarxa

This one line change makes an if inside a for loop (in realtime_peer) check all the ast_variables the loop was intending to test rather than just the first one.

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

http://svn.digium.com/view/asterisk?view=rev&revision=113012
By: Digium Subversion (svnbot) 2008-04-07 10:13:32

Repository: asterisk
Revision: 113013

_U trunk/
U trunk/channels/chan_sip.c

------------------------------------------------------------------------
r113013 | jpeeler | 2008-04-07 10:13:28 -0500 (Mon, 07 Apr 2008) | 15 lines

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

........
r113012 | jpeeler | 2008-04-07 10:16:44 -0500 (Mon, 07 Apr 2008) | 7 lines

(closes issue ASTERISK-11777)
(closes issue ASTERISK-11787)
Reported by: vinsik
Tested by: tecnoxarxa

This one line change makes an if inside a for loop (in realtime_peer) check all the ast_variables the loop was intending to test rather than just the first one.

........

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

http://svn.digium.com/view/asterisk?view=rev&revision=113013
By: Digium Subversion (svnbot) 2008-04-07 10:23:41

Repository: asterisk
Revision: 113042

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

------------------------------------------------------------------------
r113042 | jpeeler | 2008-04-07 10:23:41 -0500 (Mon, 07 Apr 2008) | 23 lines

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

................
r113013 | jpeeler | 2008-04-07 10:18:10 -0500 (Mon, 07 Apr 2008) | 15 lines

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

........
r113012 | jpeeler | 2008-04-07 10:16:44 -0500 (Mon, 07 Apr 2008) | 7 lines

(closes issue ASTERISK-11777)
(closes issue ASTERISK-11787)
Reported by: vinsik
Tested by: tecnoxarxa

This one line change makes an if inside a for loop (in realtime_peer) check all the ast_variables the loop was intending to test rather than just the first one.

........

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

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

http://svn.digium.com/view/asterisk?view=rev&revision=113042
By: Jeff Peeler (jpeeler) 2008-04-07 11:39:51

I closed this issue too soon, the change made does not check if sin is NULL.
By: Digium Subversion (svnbot) 2008-04-07 16:30:04

Repository: asterisk
Revision: 113240

U branches/1.4/channels/chan_sip.c

------------------------------------------------------------------------
r113240 | jpeeler | 2008-04-07 16:30:02 -0500 (Mon, 07 Apr 2008) | 5 lines

(closes issue ASTERISK-11777) [redo of 113012]

This fixes a for loop (in realtime_peer) to check all the ast_variables the loop was intending to test rather than just the first one. The change exposed the problem of calling memcpy on a NULL pointer, in this case the passed in sockaddr_in struct which is now checked.

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

http://svn.digium.com/view/asterisk?view=rev&revision=113240
By: Digium Subversion (svnbot) 2008-04-07 16:31:02

Repository: asterisk
Revision: 113241

U trunk/channels/chan_sip.c

------------------------------------------------------------------------
r113241 | jpeeler | 2008-04-07 16:31:01 -0500 (Mon, 07 Apr 2008) | 23 lines

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

................
r113013 | jpeeler | 2008-04-07 10:18:10 -0500 (Mon, 07 Apr 2008) | 15 lines

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

........
r113012 | jpeeler | 2008-04-07 10:16:44 -0500 (Mon, 07 Apr 2008) | 7 lines

(closes issue ASTERISK-11777)
(closes issue ASTERISK-11787)
Reported by: vinsik
Tested by: tecnoxarxa

This one line change makes an if inside a for loop (in realtime_peer) check all the ast_variables the loop was intending to test rather than just the first one.

........

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

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

http://svn.digium.com/view/asterisk?view=rev&revision=113241
By: Digium Subversion (svnbot) 2008-04-07 16:31:58

Repository: asterisk
Revision: 113242

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

------------------------------------------------------------------------
r113242 | jpeeler | 2008-04-07 16:31:57 -0500 (Mon, 07 Apr 2008) | 31 lines

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

................
r113241 | jpeeler | 2008-04-07 16:35:48 -0500 (Mon, 07 Apr 2008) | 23 lines

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

................
r113013 | jpeeler | 2008-04-07 10:18:10 -0500 (Mon, 07 Apr 2008) | 15 lines

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

........
r113012 | jpeeler | 2008-04-07 10:16:44 -0500 (Mon, 07 Apr 2008) | 7 lines

(closes issue ASTERISK-11777)
(closes issue ASTERISK-11787)
Reported by: vinsik
Tested by: tecnoxarxa

This one line change makes an if inside a for loop (in realtime_peer) check all the ast_variables the loop was intending to test rather than just the first one.

........

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

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

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

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