[Home]

Summary:ASTERISK-19182: Crash in ast_channel_get_full() with partial name
Reporter:Matthias Urlichs (smurfix)Labels:
Date Opened:2012-01-11 03:16:18.000-0600Date Closed:2012-01-11 13:24:01.000-0600
Priority:MajorRegression?Yes
Status:Closed/CompleteComponents:Core/Channels
Versions:SVN Frequency of
Occurrence		Constant
Related
Issues:
Environment:Debian Wheezy, SIP/IAX2Attachments:( 0) gdb.txt
Description:Today I saw this crash:

#0  0x080a7417 in ast_strlen_zero (s=0x2f <Address 0x2f out of bounds>)
   at /daten/src/svn/asterisk/include/asterisk/strings.h:67
#1  ast_channel_by_name_cb (obj=0xb3c6e460, arg=0xb55871b0, data=0xb558718c, flags=0) at channel.c:1428
#2  0x0808899c in internal_ao2_callback (c=0xa1cc610, flags=0, cb_fn=<optimized out>, arg=0xb55871b0,
   data=0xb558718c, type=WITH_DATA, tag=0x0, file=0x0, line=0, funcname=0x0) at astobj2.c:683
#3  0x08088ddb in __ao2_callback_data (c=0xa1cc610, flags=0, cb_fn=0x80a73f0 <ast_channel_by_name_cb>,
   arg=0xb55871b0, data=0xb558718c) at astobj2.c:801
#4  0x080aa660 in ast_channel_callback (cb_fn=0x80a73f0 <ast_channel_by_name_cb>, arg=0xb55871b0,
   data=0xb558718c, ao2_flags=0) at channel.c:1418
#5  0x080aa6aa in ast_channel_get_full (name=0xb55871b0 "IAX2/smurf-", name_len=11, exten=<optimized out>,
   context=0x0) at channel.c:1595

Context:
(gdb) fr 5
#5  0x080aa6aa in ast_channel_get_full (name=0xb55871b0 "IAX2/smurf-", name_len=11, exten=<optimized out>,
   context=0x0) at channel.c:1595
1595            } else if ((chan = ast_channel_callback(ast_channel_by_name_cb, l_name, &name_len,
(gdb) l
1590            char *l_context = (char *) context;
1591
1592            if (ast_strlen_zero(name)
1593                    && (chan = ast_channel_callback(ast_channel_by_exten_cb, l_context, l_exten, 0))) {
1594                    return chan;
1595            } else if ((chan = ast_channel_callback(ast_channel_by_name_cb, l_name, &name_len,
1596                    (name_len == 0) /* optimize if it is a complete name match */ ? OBJ_KEY : 0))) {
1597                    return chan;
1598            }
1599

Ultimately, ast_channel_by_name_cb() does this:
1424            const char *name = (flags & OBJ_KEY) ? arg : ast_channel_name((struct ast_channel *)arg);

It seems that when OBJ_KEY is set, the argument is supposed to be a channel struct, not a pointer to a name. Thus the ?: in line 1424seems to be backwards.
Comments:By: Matt Jordan (mjordan) 2012-01-11 08:22:46.422-0600

Thank you for your bug report. In order to move your issue forward, we require a backtrace[1] from the core file produced after the crash. Also, be sure you have DONT_OPTIMIZE enabled in menuselect within the Compiler Flags section, then:

make install

After enabling, reproduce the crash, and then execute the backtrace[1] instructions. When complete, attach that file to this issue report.

[1] https://wiki.asterisk.org/wiki/display/AST/Getting+a+Backtrace

While the backtrace in your comment gives a good starting point, a full backtrace for this issue would be useful.
By: Matthias Urlichs (smurfix) 2012-01-11 10:42:12.810-0600

backtrace attached.
By: Terry Wilson (twilson) 2012-01-11 13:22:48.409-0600

Thanks. Should be fixed in r350365. It isn't that OBJ_KEY should be treated as the struct ast_channel, it is that it shouldn't. When optimizing the lookup for a full name lookup (passing name_len = 0) we pass OBJ_KEY so that the hash function is used to speed up the search. When name_len != 0, we don't pass OBJ_KEY and the code was treating anything without OBJ_KEY as a struct (which is the old way that we searched).
By: Matthias Urlichs (smurfix) 2012-01-11 13:56:08.280-0600

OK. Thanks for the prompt response. I'd like to suggest a minor improvement to your patch:

+ if ((!name_len && strcasecmp(ast_channel_name(chan), name)) || (name_len && strncasecmp(ast_channel_name(chan), name, name_len))) {

That's not very readable (plus, as you noted, it's prone to mismatched parentheses); besides, a naked str*cmp always makes me wonder whether somebody forgot a bang.
IMHO it's better to use ?: here, and add a small comment:

if (name_len ? strncasecmp(ast_channel_name(chan), name, name_len) : strcasecmp(ast_channel_name(chan), name)) { /* no match */