[Asterisk-code-review] main/dial: Protect access to the format cap structure of the... (asterisk[master])

[Matt Jordan]

Matt Jordan has submitted this change and it was merged.

Change subject: main/dial: Protect access to the format_cap structure of the requesting channel
......................................................................


main/dial: Protect access to the format_cap structure of the requesting channel

When a dial attempt is made that involves a requesting channel, we previously
were not:
a) Protecting access to the native format capabilities structure on the
   requesting channel. That is inherently unsafe.
b) Reference bumping the lifetime of the format capabilities structure.

In both cases, something else could sneak in, blow away the format
capabilities, and we'd be holding onto an invalid format_cap structure. When
the newly created channel attempts to construct its format capabilities, things
go poorly.

This patch:
a) Ensures that we get a reference to the native format capabilities while
   the requesting channel is locked
b) Holds a reference to the native format capabilities during the creation
   of the new channel.

ASTERISK-25522 #close

Change-Id: I0bfb7ba8b9711f4158cbeaae96edf9626e88a54f
---
M main/dial.c
1 file changed, 5 insertions(+), 2 deletions(-)

Approvals:
  Anonymous Coward #1000019: Verified
  Matt Jordan: Looks good to me, approved
  Joshua Colp: Looks good to me, but someone else must approve



diff --git a/main/dial.c b/main/dial.c
index 34d2f70..127f327 100644
--- a/main/dial.c
+++ b/main/dial.c
@@ -295,6 +295,7 @@
 	char numsubst[AST_MAX_EXTENSION];
 	struct ast_format_cap *cap_all_audio = NULL;
 	struct ast_format_cap *cap_request;
+	struct ast_format_cap *requester_cap = NULL;
 	struct ast_assigned_ids assignedids = {
 		.uniqueid = channel->assignedid1,
 		.uniqueid2 = channel->assignedid2,
@@ -305,6 +306,7 @@
 
 		ast_channel_lock(chan);
 		max_forwards = ast_max_forwards_get(chan);
+		requester_cap = ao2_bump(ast_channel_nativeformats(chan));
 		ast_channel_unlock(chan);
 
 		if (max_forwards <= 0) {
@@ -318,8 +320,8 @@
 
 	if (cap && ast_format_cap_count(cap)) {
 		cap_request = cap;
-	} else if (chan) {
-		cap_request = ast_channel_nativeformats(chan);
+	} else if (requester_cap) {
+		cap_request = requester_cap;
 	} else {
 		cap_all_audio = ast_format_cap_alloc(AST_FORMAT_CAP_FLAG_DEFAULT);
 		ast_format_cap_append_by_type(cap_all_audio, AST_MEDIA_TYPE_AUDIO);
@@ -332,6 +334,7 @@
 		return -1;
 	}
 	cap_request = NULL;
+	ao2_cleanup(requester_cap);
 	ao2_cleanup(cap_all_audio);
 
 	if (chan) {

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

Gerrit-MessageType: merged
Gerrit-Change-Id: I0bfb7ba8b9711f4158cbeaae96edf9626e88a54f
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Matt Jordan <mjordan at digium.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Matt Jordan <mjordan at digium.com>