[Asterisk-code-review] res_musiconhold.c: Prevent crash with realtime MoH (asterisk[13])

[Kevin Harwell]

Kevin Harwell has posted comments on this change. ( https://gerrit.asterisk.org/c/asterisk/+/14684 )

Change subject: res_musiconhold.c: Prevent crash with realtime MoH
......................................................................


Patch Set 2: Code-Review-1

(3 comments)

The current approach seems incorrect. A vector is not an ao2_object, so you won't be able to use ao2 type functions on it. Do you not want the vector to potentially change while accessing it. IF so then I think AST_VECTOR_RW will be better to use here, and lock where appropriate.

https://gerrit.asterisk.org/c/asterisk/+/14684/2/res/res_musiconhold.c 
File res/res_musiconhold.c:

https://gerrit.asterisk.org/c/asterisk/+/14684/2/res/res_musiconhold.c@334 
PS2, Line 334: 	ao2_lock(state->class);
             : 	files = ao2_bump(state->class->files);
             : 	ao2_unlock(state->class);
state->class-files is a vector, and not an ao2 object. I'd expect the "bump" to assert.


https://gerrit.asterisk.org/c/asterisk/+/14684/2/res/res_musiconhold.c@348 
PS2, Line 348: 	} else if (state->save_pos >= 0 && state->save_pos < file_count && !strcmp(AST_VECTOR_GET(files, state->save_pos), state->save_pos_filename)) {
A reference of the vector still won't protect r/w conflicts. I think you need to make the vector an AST_VECTOR_RW?


https://gerrit.asterisk.org/c/asterisk/+/14684/2/res/res_musiconhold.c@403 
PS2, Line 403: 	ao2_ref(files, -1);
files is not an ao2_object so this will assert or cause problems.



-- 
To view, visit https://gerrit.asterisk.org/c/asterisk/+/14684
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Change-Id: I479c5dcf88db670956e8cac177b5826c986b0217
Gerrit-Change-Number: 14684
Gerrit-PatchSet: 2
Gerrit-Owner: Sean Bright <sean.bright at gmail.com>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-Comment-Date: Mon, 27 Jul 2020 21:04:14 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20200727/b0eaeb85/attachment.html>