[asterisk-commits] mmichelson: branch 1.6.1 r260354 - in /branches/1.6.1: ./ res/res_musiconhold.c

[SVN commits to the Asterisk project]

Author: mmichelson
Date: Fri Apr 30 15:18:31 2010
New Revision: 260354

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=260354
Log:
Merged revisions 260346 via svnmerge from 
https://origsvn.digium.com/svn/asterisk/trunk

................
  r260346 | mmichelson | 2010-04-30 15:11:02 -0500 (Fri, 30 Apr 2010) | 24 lines
  
  Merged revisions 260345 via svnmerge from 
  https://origsvn.digium.com/svn/asterisk/branches/1.4
  
  ........
    r260345 | mmichelson | 2010-04-30 15:08:15 -0500 (Fri, 30 Apr 2010) | 18 lines
    
    Fix potential crash from race condition due to accessing channel data without the channel locked.
    
    In res_musiconhold.c, there are several places where a channel's
    stream's existence is checked prior to calling ast_closestream on it. The issue
    here is that in several cases, the channel was not locked while checking the
    stream. The result was that if two threads checked the state of the channel's
    stream at approximately the same time, then there could be a situation where
    both threads attempt to call ast_closestream on the channel's stream. The result
    here is that the refcount for the stream would go below 0, resulting in a crash.
    
    I have added proper channel locking to res_musiconhold.c to ensure that
    we do not try to check chan->stream without the channel locked. A Digium customer
    has been using this patch for several weeks and has not had any crashes since
    applying the patch.
    
    ABE-2147
  ........
................

Modified:
    branches/1.6.1/   (props changed)
    branches/1.6.1/res/res_musiconhold.c

Propchange: branches/1.6.1/
------------------------------------------------------------------------------
Binary property 'trunk-merged' - no diff available.

Modified: branches/1.6.1/res/res_musiconhold.c
URL: http://svnview.digium.com/svn/asterisk/branches/1.6.1/res/res_musiconhold.c?view=diff&rev=260354&r1=260353&r2=260354
==============================================================================
--- branches/1.6.1/res/res_musiconhold.c (original)
+++ branches/1.6.1/res/res_musiconhold.c Fri Apr 30 15:18:31 2010
@@ -328,7 +328,15 @@
 	state->sample_queue += samples;
 
 	while (state->sample_queue > 0) {
+		ast_channel_lock(chan);
 		if ((f = moh_files_readframe(chan))) {
+			/* We need to be sure that we unlock
+			 * the channel prior to calling
+			 * ast_write. Otherwise, the recursive locking
+			 * that occurs can cause deadlocks when using
+			 * indirect channels, like local channels
+			 */
+			ast_channel_unlock(chan);
 			state->samples += f->samples;
 			state->sample_queue -= f->samples;
 			res = ast_write(chan, f);
@@ -337,8 +345,10 @@
 				ast_log(LOG_WARNING, "Failed to write frame to '%s': %s\n", chan->name, strerror(errno));
 				return -1;
 			}
-		} else
+		} else {
+			ast_channel_unlock(chan);
 			return -1;	
+		}
 	}
 	return res;
 }
@@ -1422,11 +1432,11 @@
 
 static void local_ast_moh_stop(struct ast_channel *chan)
 {
-	struct moh_files_state *state = chan->music_state;
 	ast_clear_flag(chan, AST_FLAG_MOH);
 	ast_deactivate_generator(chan);
 
-	if (state) {
+	ast_channel_lock(chan);
+	if (chan->music_state) {
 		if (chan->stream) {
 			ast_closestream(chan->stream);
 			chan->stream = NULL;
@@ -1438,6 +1448,7 @@
 		"Channel: %s\r\n"
 		"UniqueID: %s\r\n",
 		chan->name, chan->uniqueid);
+	ast_channel_unlock(chan);
 }
 
 static void moh_class_destructor(void *obj)