[Home]

Summary:ASTERISK-18346: MusicOnHold has extra unref which may lead to memory corruption and crash
Reporter:Mark Murawski (kobaz)Labels:
Date Opened:2011-08-25 12:14:48Date Closed:2011-09-02 17:18:47
Priority:MajorRegression?
Status:Closed/CompleteComponents:Resources/res_musiconhold
Versions:1.8.5.0 Frequency of
Occurrence		Constant
Related
Issues:
Environment:Attachments:( 0) jira_asterisk_18346_v1.8.patch
Description:musiconhold.conf
----------------
[ring]
mode=files
directory=/var/lib/asterisk/mohring
----------------

Dialplan
------------------------
Dial(SIP/2604,,m(ring));
------------------------

Call 2604
2604 picks up
2604 holds
2604 unholds
2604 hangs up

If you have REF_DEBUG enabled:
{noformat}
0xb6892b10 +1   res_musiconhold.c:1341:local_ast_moh_start (get_mohbyname) [@1]
0xb6892b10 -1   res_musiconhold.c:1515:local_ast_moh_start (unreffing local reference to mohclass in local_ast_moh_start) [@4]
0xb6893438 +1   res_musiconhold.c:1329:local_ast_moh_start (get_mohbyname) [@1]
0xb6893438 -1   res_musiconhold.c:1515:local_ast_moh_start (unreffing local reference to mohclass in local_ast_moh_start) [@4]
0xb6893438 -1   res_musiconhold.c:279:moh_files_release (Unreffing channel's music class upon deactivation of generator) [@3]
0xb6892b10 -1   res_musiconhold.c:909:moh_release (unreffing moh->parent upon deactivation of generator) [@3]
0xb6892b10 -1   res_musiconhold.c:1276:local_ast_moh_cleanup (Channel MOH state destruction) [@2]
0xb6892b10 =1   res_musiconhold.c:1276:local_ast_moh_cleanup (Channel MOH state destruction) BAD ATTEMPT!
0xb6893438 +1   res_musiconhold.c:1329:local_ast_moh_start (get_mohbyname) [@1]
{noformat}

And you'll also get
[Aug 25 13:13:39] DEBUG[27731]: res_musiconhold.c:242 _mohclass_unref: Attempt to unref mohclass 0xb70722e0 (default) when only 1 ref remained, and class is still in a container! (at res_musiconhold.c:1276 (local_ast_moh_cleanup))
Comments:By: Mark Murawski (kobaz) 2011-08-26 19:38:14.981-0500

Pretty sure the reviewboard patch fixes the problem.  Anyone interested in testing/confirming?
By: Richard Mudgett (rmudgett) 2011-08-31 20:25:37.278-0500

The [^jira_asterisk_18346_v1.8.patch] resolves the reference inconsistencies in moh_alloc() and moh_release().

See
https://reviewboard.asterisk.org/r/1404/
By: Richard Mudgett (rmudgett) 2011-08-31 21:09:27.028-0500

Updated to add another warning message.
By: Mark Murawski (kobaz) 2011-09-02 14:13:57.486-0500

Looks good.  Testing shows this patch fixes it.
By: Richard Mudgett (rmudgett) 2011-09-02 14:39:43.520-0500

Mark please mark your original reviewboard patch as discarded.
By: Mark Murawski (kobaz) 2011-09-02 14:42:02.401-0500

Done.
By: Richard Mudgett (rmudgett) 2011-09-02 17:18:26.972-0500

Committed fix:
r334358 | rmudgett | 2011-09-02 16:09:31 -0500 (Fri, 02 Sep 2011) | 33 lines
Changed paths:
  M /trunk
  M /trunk/res/res_musiconhold.c

Merged revisions 334357 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/10

................
 r334357 | rmudgett | 2011-09-02 16:08:16 -0500 (Fri, 02 Sep 2011) | 26 lines

 Merged revisions 334355 via svnmerge from
 https://origsvn.digium.com/svn/asterisk/branches/1.8

 ........
   r334355 | rmudgett | 2011-09-02 15:59:49 -0500 (Fri, 02 Sep 2011) | 19 lines

   MusicOnHold has extra unref which may lead to memory corruption and crash.

   The problem happens when a call is disconnected and you had started a MOH
   class that does not use the files mode.  If you define REF_DEBUG and
   recreate the problem, it will announce itself with the following warning:
   Attempt to unref mohclass 0xb70722e0 (default) when only 1 ref remained,
   and class is still in a container!

   * Fixed moh_alloc() and moh_release() functions not handling the
   state->class reference consistently.

   (closes issue ASTERISK-18346)
   Reported by: Mark Murawski
   Patches:
         jira_asterisk_18346_v1.8.patch (license #5621) patch uploaded by rmudgett
   Tested by: rmudgett, Mark Murawski

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