[Asterisk-code-review] Stasis: Prevent operations on hung-up channels. (asterisk[13])

[Mark Michelson]

Mark Michelson has uploaded a new change for review.

  https://gerrit.asterisk.org/2053

Change subject: Stasis: Prevent operations on hung-up channels.
......................................................................

Stasis: Prevent operations on hung-up channels.

When a channel enters Stasis, a stasis_app_control structure is created
and a pointer to the channel is added on that control. When the channel
exits Stasis, the control is removed from its container of controls and
presumably destroyed. Channel operations in Stasis happen on a separate
thread. The typical workflow is to grab a reference to the control
structure and then perform an operation on the control's channel.

The problem here is that there is a brief window where the thread
performing the channel operation can grab a reference to the control.
Then the channel can hang up, exit Stasis, and be freed. Then the thread
performing the channel opeation can attempt to perform the operation.
The problem here is that we're attempting an operation on a freed
channel, resulting in a crash.

This patch does two things in order to attempt to fix this
1) The control now has a reference to the channel instead of a bare
pointer. Since controls share ownership between threads, the contents of
those controls have to be treated the same way. With this change, we
will not allow for the channel to be freed out from under the control
structure.
2) When getting the control structure, we will attempt to see if the
channel has hung up. This can help to short-circuit operations that are
performed on hung-up channels, resulting in a 404 return instead of an
erroneous 200-class return.

ASTERISK-25709 #close
Reported by Mark Michelson

Change-Id: I3b114e6019a3ab9337098039a2a114f7accebb52
---
M include/asterisk/stasis_app.h
M res/ari/resource_channels.c
M res/stasis/control.c
M res/stasis/control.h
4 files changed, 20 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/53/2053/1

diff --git a/include/asterisk/stasis_app.h b/include/asterisk/stasis_app.h
index f2b07e0..89e2a6a 100644
--- a/include/asterisk/stasis_app.h
+++ b/include/asterisk/stasis_app.h
@@ -862,6 +862,12 @@
  */
 int stasis_app_channel_set_internal(struct ast_channel *chan);
 
+/*!
+ * \brief Retrieve the channel from a control structure
+ *
+ * \return The channel that the control was created for
+ */
+struct ast_channel *stasis_app_control_channel(struct stasis_app_control *control);
 /*! @} */
 
 #endif /* _ASTERISK_STASIS_APP_H */
diff --git a/res/ari/resource_channels.c b/res/ari/resource_channels.c
index f722802..1bc97c8 100644
--- a/res/ari/resource_channels.c
+++ b/res/ari/resource_channels.c
@@ -84,6 +84,13 @@
 		return NULL;
 	}
 
+	/* Even if we can find the control, the channel may have hung up */
+	if (ast_check_hangup(stasis_app_control_channel(control))) {
+		ast_ari_response_error(response, 404, "Not Found",
+				"Channel not found");
+		return NULL;
+	}
+
 	ao2_ref(control, +1);
 	return control;
 }
diff --git a/res/stasis/control.c b/res/stasis/control.c
index 87362df..fc84678 100644
--- a/res/stasis/control.c
+++ b/res/stasis/control.c
@@ -97,6 +97,7 @@
 	ao2_cleanup(control->command_queue);
 	ast_cond_destroy(&control->wait_cond);
 	ao2_cleanup(control->app);
+	ast_channel_cleanup(control->channel);
 }
 
 struct stasis_app_control *control_create(struct ast_channel *channel, struct stasis_app *app)
@@ -125,7 +126,7 @@
 		return NULL;
 	}
 
-	control->channel = channel;
+	control->channel = ao2_bump(channel);
 
 	AST_LIST_HEAD_INIT(&control->add_rules);
 	AST_LIST_HEAD_INIT(&control->remove_rules);
@@ -1084,3 +1085,8 @@
 {
 	return control->app;
 }
+
+struct ast_channel *stasis_app_control_channel(struct stasis_app_control *control)
+{
+	return control->channel;
+}
diff --git a/res/stasis/control.h b/res/stasis/control.h
index a139f82..a378836 100644
--- a/res/stasis/control.h
+++ b/res/stasis/control.h
@@ -108,5 +108,4 @@
 	struct stasis_app_control *control,
 	struct ast_channel *chan, void *obj);
 
-
 #endif /* _ASTERISK_RES_STASIS_CONTROL_H */

-- 
To view, visit https://gerrit.asterisk.org/2053
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I3b114e6019a3ab9337098039a2a114f7accebb52
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Owner: Mark Michelson <mmichelson at digium.com>