[asterisk-commits] mjordan: trunk r428866 - in /trunk: ./ apps/app_voicemail.c

[SVN commits to the Asterisk project]

Author: mjordan
Date: Wed Dec  3 10:45:24 2014
New Revision: 428866

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=428866
Log:
apps/app_voicemail: Fix crash with IMAP when streams are opened simultaneously

The UW IMAP library is instrinsically not thread-safe, and relies upon higher
level applications to guarantee thread safety. For the most part, this is
provided by the vms object, which provides locking for individual streams.
Unfortunately, this is not sufficient for calls to mail_open which create the
IMAP stream. mail_open can, on some systems, call into a UW IMAP specific
function for determining the address of a system based on a hostname,
ip_nametoaddr.

In the ip6_unix implementation of this function, static variables are used
to hold parsing buffers. This can cause a crash if multiple threads attempt
to convert a hostname to an address at the same time. Locking on a single
mail stream is not sufficient to prevent simultaneous access to these static
variables.

In the IMAP library, this function can be called from the mail_open and
imap_status functions. As the imap_status function is not used by
app_voicemail, locking on access to mail_open is sufficient to prevent
any mangling of the buffers.

Review: https://reviewboard.asterisk.org/r/4188/

ASTERISK-24516 #close
Reported by: David Duncan Ross Palmer
Tested by: David Duncan Ross Palmer
patches:
  ASTERISK-24516.diff uploaded by David Duncan Ross Palmer (License 6660)
........

Merged revisions 428863 from http://svn.asterisk.org/svn/asterisk/branches/11
........

Merged revisions 428864 from http://svn.asterisk.org/svn/asterisk/branches/12
........

Merged revisions 428865 from http://svn.asterisk.org/svn/asterisk/branches/13

Modified:
    trunk/   (props changed)
    trunk/apps/app_voicemail.c

Propchange: trunk/
------------------------------------------------------------------------------
Binary property 'branch-13-merged' - no diff available.

Modified: trunk/apps/app_voicemail.c
URL: http://svnview.digium.com/svn/asterisk/trunk/apps/app_voicemail.c?view=diff&rev=428866&r1=428865&r2=428866
==============================================================================
--- trunk/apps/app_voicemail.c (original)
+++ trunk/apps/app_voicemail.c Wed Dec  3 10:45:24 2014
@@ -510,6 +510,9 @@
 static int imapgreetings = 0;
 static char delimiter = '\0';
 
+/* mail_open cannot be protected on a stream basis */
+ast_mutex_t mail_open_lock;
+
 struct vm_state;
 struct ast_vm_user;
 
@@ -2955,7 +2958,9 @@
 		/* Connect to INBOX first to get folders delimiter */
 		imap_mailbox_name(tmp, sizeof(tmp), vms, 0, 1);
 		ast_mutex_lock(&vms->lock);
+		ast_mutex_lock(&mail_open_lock);
 		stream = mail_open (stream, tmp, debug ? OP_DEBUG : NIL);
+		ast_mutex_unlock(&mail_open_lock);
 		ast_mutex_unlock(&vms->lock);
 		if (stream == NIL) {
 			ast_log(LOG_ERROR, "Can't connect to imap server %s\n", tmp);
@@ -2971,7 +2976,9 @@
 	imap_mailbox_name(tmp, sizeof(tmp), vms, box, 1);
 	ast_debug(3, "Before mail_open, server: %s, box:%d\n", tmp, box);
 	ast_mutex_lock(&vms->lock);
+	ast_mutex_lock(&mail_open_lock);
 	vms->mailstream = mail_open (stream, tmp, debug ? OP_DEBUG : NIL);
+	ast_mutex_unlock(&mail_open_lock);
 	ast_mutex_unlock(&vms->lock);
 	if (vms->mailstream == NIL) {
 		return -1;