[Home]

Summary:ASTERISK-14370: [patch] 1.6.1.1: Memory handling error in main/pbx.c (pbx_extension_helper)
Reporter:Yurii Rashkovskii (yrashk)Labels:
Date Opened:2009-06-24 19:25:32Date Closed:2009-11-11 14:02:18.000-0600
Priority:MinorRegression?No
Status:Closed/CompleteComponents:PBX/General
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) sol10gdb.txt
( 1) sol10segfault.patch
Description:Due to Solaris x64 specifics, it won't accept NULLs as string. I think Asterisk had similar problems before, and apparently it is still doing this in some places. I found one of them today, since it was blocking asterisk to start.

Basically, asterisk was segfaulting on every start, on the strlen() routine.

Attached you can find gdb analysis on a core dumped and a patch that solves this problem

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

% uname -a
SunOS pbx0 5.10 Generic_139556-08 i86pc i386 i86pc
% cat /etc/release
                       Solaris 10 5/09 s10x_u7wos_08 X86
          Copyright 2009 Sun Microsystems, Inc.  All Rights Reserved.
                       Use is subject to license terms.
                            Assembled 30 March 2009
Comments:By: Yurii Rashkovskii (yrashk) 2009-06-24 19:28:42

The patch is:

--- old/asterisk-1.6.1.1/main/pbx.c Fri Apr 17 10:33:27 2009
+++ asterisk-1.6.1.1/main/pbx.c Thu Jun 25 16:31:23 2009
@@ -3143,6 +3143,9 @@

int matching_action = (action == E_MATCH || action == E_CANMATCH || action == E_MATCHMORE);

+
+        context = context ? context : ""; /* Context should not be NULL */
+
ast_rdlock_contexts();
if (found)
*found = 0;
By: Yurii Rashkovskii (yrashk) 2009-06-24 19:34:06

I realize that there might be a better way to handle this; I am not very familiar with asterisk codebase. Will update this issue if I'll get it fixed in a better way.


By: snuffy (snuffy) 2009-07-12 20:56:29

Yes yrashk your right.
Solaris / OpenSolaris does not support NULL to printf.
I did a very small example program and found that strlen does not cause issue.
See below:

#include <stdio.h>
#include <stdlib.h>
#include <string.h>

int main (int argc, char **argv)
{
  char *a = "stuff";
  char *b = "";
  char *c = NULL;

  if (argc == 2)
     printf("a:%s,b:%s,c:%s",a,b,c);
  else
     strlen(c);

  return 0;
}


By: Yurii Rashkovskii (yrashk) 2009-07-12 21:50:05

It looks like some use of strlen() from within printf() causes the problem, I just ran gdb on the core dump your program generated:

(gdb) bt
#0  0xfeea598c in strlen () from /lib/libc.so.1
#1  0xfef00432 in _ndoprnt () from /lib/libc.so.1
#2  0xfef030f0 in printf () from /lib/libc.so.1
#3  0x080508b6 in main ()

That's basically why I thought strlen() is causing the problem


By: snuffy (snuffy) 2009-07-13 00:01:52

No problems.

We should still think about fixing this across the board. Its bad/lazy coding to pass null into printf.

Maybe could be janitor project worthy, basically it could result in some extra code though, all args before going to be printed should be checked.
By: Leif Madsen (lmadsen) 2009-07-29 11:14:42

If this is just a janitor project, should this issue be closed, and some text document updated somewhere?
By: Mark Michelson (mmichelson) 2009-07-30 09:48:47

Even if there is a janitor project created, the initial fix for this specific issue should be made before closing the issue. Keeping this issue open until every single potential instance of this is fixed is excessive in my opinion.
By: snuffy (snuffy) 2009-07-30 18:10:07

I'm not saying that this proposed fix cannot go in, my note was more in passing.

I have no issue with making a null string of context, it seems to happen on loading of the module, i guess in that circumstance you may end up calling something like pbx_extension_helper with a NULL context.

Maybe a deeper fix of checking for NULL strings inside ast_log calls could also be an answer to this?


By: Digium Subversion (svnbot) 2009-11-11 13:52:19.000-0600

Repository: asterisk
Revision: 229498

U   branches/1.4/main/pbx.c

------------------------------------------------------------------------
r229498 | dbrooks | 2009-11-11 13:52:18 -0600 (Wed, 11 Nov 2009) | 8 lines

Solaris doesn't like NULL going to ast_log

Solaris will crash if NULL is passed to ast_log. This simple patch simply uses S_OR to
get around this.

(closes issue ASTERISK-14370)
Reported by: yrashk

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

http://svn.digium.com/view/asterisk?view=rev&revision=229498
By: Digium Subversion (svnbot) 2009-11-11 13:54:18.000-0600

Repository: asterisk
Revision: 229499

_U  trunk/
U   trunk/main/pbx.c

------------------------------------------------------------------------
r229499 | dbrooks | 2009-11-11 13:54:17 -0600 (Wed, 11 Nov 2009) | 15 lines

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

........
 r229498 | dbrooks | 2009-11-11 13:46:19 -0600 (Wed, 11 Nov 2009) | 8 lines
 
 Solaris doesn't like NULL going to ast_log
 
 Solaris will crash if NULL is passed to ast_log. This simple patch simply uses S_OR to
 get around this.
 
 (closes issue ASTERISK-14370)
 Reported by: yrashk
........

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

http://svn.digium.com/view/asterisk?view=rev&revision=229499
By: Digium Subversion (svnbot) 2009-11-11 13:57:46.000-0600

Repository: asterisk
Revision: 229500

U   branches/1.6.0/main/pbx.c

------------------------------------------------------------------------
r229500 | dbrooks | 2009-11-11 13:57:45 -0600 (Wed, 11 Nov 2009) | 22 lines

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

................
 r229499 | dbrooks | 2009-11-11 13:48:18 -0600 (Wed, 11 Nov 2009) | 15 lines
 
 Merged revisions 229498 via svnmerge from
 https://origsvn.digium.com/svn/asterisk/branches/1.4
 
 ........
   r229498 | dbrooks | 2009-11-11 13:46:19 -0600 (Wed, 11 Nov 2009) | 8 lines
   
   Solaris doesn't like NULL going to ast_log
   
   Solaris will crash if NULL is passed to ast_log. This simple patch simply uses S_OR to
   get around this.
   
   (closes issue ASTERISK-14370)
   Reported by: yrashk
 ........
................

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

http://svn.digium.com/view/asterisk?view=rev&revision=229500
By: Digium Subversion (svnbot) 2009-11-11 14:00:29.000-0600

Repository: asterisk
Revision: 229501

_U  branches/1.6.1/
U   branches/1.6.1/main/pbx.c

------------------------------------------------------------------------
r229501 | dbrooks | 2009-11-11 14:00:28 -0600 (Wed, 11 Nov 2009) | 22 lines

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

................
 r229499 | dbrooks | 2009-11-11 13:48:18 -0600 (Wed, 11 Nov 2009) | 15 lines
 
 Merged revisions 229498 via svnmerge from
 https://origsvn.digium.com/svn/asterisk/branches/1.4
 
 ........
   r229498 | dbrooks | 2009-11-11 13:46:19 -0600 (Wed, 11 Nov 2009) | 8 lines
   
   Solaris doesn't like NULL going to ast_log
   
   Solaris will crash if NULL is passed to ast_log. This simple patch simply uses S_OR to
   get around this.
   
   (closes issue ASTERISK-14370)
   Reported by: yrashk
 ........
................

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

http://svn.digium.com/view/asterisk?view=rev&revision=229501
By: Digium Subversion (svnbot) 2009-11-11 14:02:17.000-0600

Repository: asterisk
Revision: 229502

_U  branches/1.6.2/
U   branches/1.6.2/main/pbx.c

------------------------------------------------------------------------
r229502 | dbrooks | 2009-11-11 14:02:17 -0600 (Wed, 11 Nov 2009) | 22 lines

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

................
 r229499 | dbrooks | 2009-11-11 13:48:18 -0600 (Wed, 11 Nov 2009) | 15 lines
 
 Merged revisions 229498 via svnmerge from
 https://origsvn.digium.com/svn/asterisk/branches/1.4
 
 ........
   r229498 | dbrooks | 2009-11-11 13:46:19 -0600 (Wed, 11 Nov 2009) | 8 lines
   
   Solaris doesn't like NULL going to ast_log
   
   Solaris will crash if NULL is passed to ast_log. This simple patch simply uses S_OR to
   get around this.
   
   (closes issue ASTERISK-14370)
   Reported by: yrashk
 ........
................

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

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