[Home]

Summary:ASTERISK-04678: [patch] Enter wrong voicemail password will make Asterisk segfault
Reporter:philou (philou)Labels:
Date Opened:2005-07-26 05:18:32Date Closed:2008-01-15 15:43:45.000-0600
Priority:CriticalRegression?No
Status:Closed/CompleteComponents:Applications/app_voicemail
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) Asterisk-vm-authenticate.backtrace
( 1) voicemail.patch
( 2) voicemail2.patch
( 3) voicemail3.patch
Description:When authentication password is wrong , in the voicemail, Asterisk will segfault

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

Reproduced on two different servers
Backtrace provided (thread apply all bt full)
Comments:By: Russell Bryant (russell) 2005-07-26 11:36:26

ack!  weirdness!  I was able to reproduce the segfault.  Then, I did a "make dont-optimize" build so I could get a clearer backtrace and now it doesn't crash.

Can you try to do a non-optimized build and see if you can still recreate it and post an updated backtrace?
By: philou (philou) 2005-07-26 14:15:11

Well, the trace i posted initially, was made with "make dont-optimize", and segfault caught up under Valgrind

What else can i do to help you out with this issue ? This is 100% reproductible here. Funny still, on the first box, the full backtrace is only few lines long
the longer trace was caught on the second box.
By: Joe Antkowiak (antkojm1) 2005-07-28 03:25:08

reproduced here as well.  Seems to be broken after the update on 7/11/05:

* 07-11 - 2005 : An issue with voicemail synchronization has been fixed
By: Clod Patry (junky) 2005-07-28 19:34:54

I'd like to see your voicemail.conf, and all ur CLI's output (until it crashes).
By: nick (nick) 2005-07-28 20:50:14

I don't know that this is a complete fix (as in, do we even need to call close_mailbox() if we weren't able to authenticate? I don't think that we do...), but this fixes the segfault and adds a check that probably should be there anyway.

DonF

Nick
By: philou (philou) 2005-07-30 08:53:31

Well, i should add that i first need to delete the .lock* files in / before i start Asterisk otherwise it won't segfault, i'll just have warning:

Jul 30 15:38:55 WARNING[14867]: app_voicemail.c:4645 vm_authenticate: Unable to read password
Jul 30 15:39:00 WARNING[14867]: app.c:1125 ast_lock_path: Failed to lock path '': File exists

But then , i delete the /.lock* files, restart * then enter wrong password :
Jul 30 15:39:52 WARNING[14899]: app_voicemail.c:4645 vm_authenticate: Unable to read password
Segmentation fault (core dumped)

BTW, the attached patch doesn't help here, i still got the segfault , with or wouthout the patch

Thanks
By: Matthew Kincaid (mkincaid) 2005-07-31 15:22:27

I get this problem also and it seems to be related to the lock file. If I give my asterisk user access to write files to / then everything appears to operate fine. Looking through the CVS history (http://cvs.digium.com/index.cgi/apps/app_voicemail.c.diff?r1=1.228;r2=1.229;f=h) My guess is it happened in changing from ast_lock_path() to vm_lock_path(), but I wasn't able to track down what exactly is happening.
By: philou (philou) 2005-07-31 19:22:47

Well, ok, i've found the exact revision where the problem happens:

Revision 1.224 of app_voicemail.c works and Revison 1.225 segfaults

This is due to the following update

* 05-11 - 2005 : An option for maximum number of messsages per mailbox added by GDS Partners (www.gdspartners.com)
* Stojan Sljivic <stojan.sljivic@gdspartners.com>

and not

* 07-11 - 2005 : An issue with voicemail synchronization has been fixed

as antkojm1 stated previously

Here's the diff:

http://cvs.digium.com/index.cgi/apps/app_voicemail.c.diff?r1=1.224;r2=1.225;f=h

Unfortunately, asterisk wont build with make dont-optimize with revision 1.225 of app_voicemail.c. Normal "make install" will build fine though

undefined references to ast_copy_string in logger.c at 3 or 4 places
By: philou (philou) 2005-07-31 20:33:39

The newly uploaded patch "works for me(tm)". No more segfault with latest CVS
By: ssljivic (ssljivic) 2005-08-01 06:00:34

Hi philou,

The patch2 is OK.
I would just add the following:

if (vms->curbox || vms->deleted || vms->heard)
return 0;

instead of

if (vms->curbox || vms->deleted || vms->heard)
return 0;

The error occurs because when a wrong pass is entered vms->deleted and vms->heard are not allocated.

You are right philou, that this is not related to the "voicemail synchronization", but rather to the "option for maximum number of messages" patch.

Thanks philou.

Regards,
Stojan Sljivic
By: ssljivic (ssljivic) 2005-08-01 06:02:27

Sorry for mistake:

...instead of

   if (vms->curbox)
       return 0;
By: Mark Spencer (markster) 2005-08-01 07:03:16

Should be fixed in latest CVS head...  Thanks!
By: philou (philou) 2005-08-01 07:38:59

Hmm, well, since i just updated to latest CVS head and that the fix hasn't still been committed, i have uploaded another patch that should be a definitive fix for that bug.

Thanks,
By: Michael Jerris (mikej) 2005-08-01 07:46:30

by the fix hasn't commited do you mean that you tested and still have the problem in current head, or that the fix you were looking for is not in the sourcecode.  If you were just looking in the source, please test against current head please.
By: philou (philou) 2005-08-01 09:30:10

Hello MikeJ

Well, i actually updated to latest CVS and i still got the segfault, that's why i thought the fix hadn't been committed , also as the fix i was looking for wasn't at the place i was expecting it.

Now it segfaults here:

#0  close_mailbox (vms=0xb762759c, vmu=0xb7627a5c) at app_voicemail.c:3578
3578 memset(vms->deleted, 0, sizeof(vms->deleted));

I believe that the fix you committed sets vms.lastmsg to -1 which makes it jump to done: in close_mailbox(), but still vms->deleted isn't defined , right ?

Thanks
By: Mark Monnin (wrmem) 2005-08-01 09:34:25

Still crashes even with markster's patch.  On my system, vms->deleted and vms->heard are NULL pointers, and dereferenced in these memsets.

done:
   memset(vms->deleted, 0, sizeof(vms->deleted));
   memset(vms->heard, 0, sizeof(vms->heard));

Here's the backtrace(gdb) bt
#0  0x00ef2c65 in close_mailbox (vms=0x2ee8cd0, vmu=0x2ee8880) at app_voicemail.c:3578
#1  0x00eeb5c5 in vm_execmain (chan=0x8255918, data=0x0) at app_voicemail.c:5120
#2  0x0808a8df in pbx_extension_helper (c=0x8255918, con=0x2ee8cd0, context=0x8255a68 "agent-login", exten=0x8255b5c "4",
   priority=1, label=0x0, callerid=0x82a7088 "VoiceMailMain", action=0) at pbx.c:553
#3  0x0808b4e4 in __ast_pbx_run (c=0x8255918) at pbx.c:2149
#4  0x0808c109 in pbx_thread (data=0x8255918) at pbx.c:2436
ASTERISK-1  0x00115de8 in start_thread () from /lib/tls/libpthread.so.0
ASTERISK-2  0x001fd93a in clone () from /lib/tls/libc.so.6
(gdb) print vms->deleted
$6 = (int *) 0x0
(gdb) print vms->heard
$7 = (int *) 0x0


XXX*CLI> show version
Asterisk CVS-HEAD built by XXX@XXX on a i686 running Linux on 2005-08-01 13:33:08 UTC
By: ssljivic (ssljivic) 2005-08-01 09:54:26

Patch 3 should fix the issue.

In patch 2 the if statement:
       if (vms->curbox || vms->deleted || vms->heard)
               return 0;
is in the wrong place.

In patch 3 it moved to the correct location.
By: Mark Spencer (markster) 2005-08-02 22:52:11

Fixed in CVS head.  Thanks!
By: Russell Bryant (russell) 2005-08-04 19:00:15

the same check committed to cvs head has been added to 1.0 as well
By: Digium Subversion (svnbot) 2008-01-15 15:43:15.000-0600

Repository: asterisk
Revision: 6250

U   trunk/apps/app_voicemail.c

------------------------------------------------------------------------
r6250 | markster | 2008-01-15 15:43:14 -0600 (Tue, 15 Jan 2008) | 2 lines

Make sure we don't close a mailbox if we didn't open one (bug ASTERISK-4678)

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

http://svn.digium.com/view/asterisk?view=rev&revision=6250
By: Digium Subversion (svnbot) 2008-01-15 15:43:25.000-0600

Repository: asterisk
Revision: 6261

U   trunk/apps/app_voicemail.c

------------------------------------------------------------------------
r6261 | markster | 2008-01-15 15:43:24 -0600 (Tue, 15 Jan 2008) | 2 lines

Fix voicemail crash (bug ASTERISK-4678)

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

http://svn.digium.com/view/asterisk?view=rev&revision=6261
By: Digium Subversion (svnbot) 2008-01-15 15:43:45.000-0600

Repository: asterisk
Revision: 6284

U   branches/v1-0/apps/app_voicemail.c

------------------------------------------------------------------------
r6284 | russell | 2008-01-15 15:43:45 -0600 (Tue, 15 Jan 2008) | 2 lines

fix potential crash in close_mailbox (bug ASTERISK-4678)

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

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