Summary: | ASTERISK-10500: [patch] Division by zero error when a music on hold class is empty | ||
Reporter: | jcomellas (jcomellas) | Labels: | |
Date Opened: | 2007-10-11 11:06:09 | Date Closed: | 2007-11-09 10:34:33.000-0600 |
Priority: | Critical | Regression? | No |
Status: | Closed/Complete | Components: | Resources/res_musiconhold |
Versions: | Frequency of Occurrence | ||
Related Issues: | |||
Environment: | Attachments: | ( 0) res_musiconhold_division_by_zero.patch | |
Description: | When a music on hold class is empty (chan->music_state->class->total_files == 0) and Asterisk looks for the next file in the music class by calling ast_moh_files_next() (on res/res_musiconhold.c) we get a division by zero signal. ****** ADDITIONAL INFORMATION ****** The crash happened to us on res_musiconhold.c, line 249, where it says: state->pos %= state->class->total_files; When state->class->total_files is 0, Asterisk tries to get the remainder of division by zero, which causes the signal to be raised. | ||
Comments: | By: Russell Bryant (russell) 2007-10-11 11:16:15 I can't actually see how total_files could ever be 0. The code for loading moh classes checks to see if no files were found, and just destroys it if that is the case. if (!moh_scan_files(moh)) { ast_moh_free_class(&moh); return -1; } By: jcomellas (jcomellas) 2007-10-11 11:18:39 When submitting the patch I was asked to sign the license agreement, which I had previously sent (almost 2 years ago) as a scanned copy of a document signed by me. By: jcomellas (jcomellas) 2007-10-11 11:44:55 Sorry, I've just realized that this had happened on Asterisk 1.2.18 instead of 1.4.13, but the code seems to be pretty similar in Asterisk 1.4. I don't know what may have caused this problem, but it did happen. The only strange thing we do regarding MOH is that we run 'moh reload' very frequently (every 5 min) and there are lots of music classes (hundreds) with files that are loaded from an NFS filesystem. The Asterisk 1.2.18 backtrace I have is the following (for a bridge that was created from a module written by us): Program terminated with signal 8, Arithmetic exception. <...List of Asterisk modules being loaded...> #0 0xb7b7fed8 in moh_files_generator (chan=0xb6cdabe8, data=0xc2da3c8, len=160, samples=160) at res_musiconhold.c:227 227 state->pos %= state->class->total_files; (gdb) bt #0 0xb7b7fed8 in moh_files_generator (chan=0xb6cdabe8, data=0xc2da3c8, len=160, samples=160) at res_musiconhold.c:227 #1 0x08066bfa in ast_read (chan=0xb6cdabe8) at channel.c:2032 #2 0x0806a02d in ast_channel_bridge (c0=0xb6cdabe8, c1=0xd7afcc0, config=0xb6662da8, fo=0xb6662d08, rc=0xb6662d04) at channel.c:3408 #3 0xb7b58671 in ast_bridge_call (chan=0xb6cdabe8, peer=0xd7afcc0, config=0xb6662da8) at res_features.c:1325 #4 0xb7560dcb in telmod_bridge (session=0xd424828, msg=0xd9cfc00) at src/method.c:1543 ASTERISK-1 0xb755b014 in telmod_event_loop (session=0xd424828) at src/event.c:214 ASTERISK-2 0xb7566e0e in telmod_exec (chan=0xb6cdabe8, data=0xb6666fd8) at src/app_telmod.c:198 ASTERISK-3 0x0808e443 in pbx_extension_helper (c=0xb6cdabe8, con=<value optimized out>, context=<value optimized out>, exten=0xb6cdae2c "DID13214451888!C542860810!S0!DIR0", priority=4, label=0x0, callerid=0xb4d05e20 "3308684196", action=1) at pbx.c:574 ASTERISK-4 0x0808f8e4 in __ast_pbx_run (c=0xb6cdabe8) at pbx.c:2250 ASTERISK-5 0x080904fc in pbx_thread (data=0x1) at pbx.c:2537 ASTERISK-6 0xb7f35341 in start_thread () from /lib/tls/i686/cmov/libpthread.so.0 ASTERISK-7 0xb7e164ee in clone () from /lib/tls/i686/cmov/libc.so.6 By: Digium Subversion (svnbot) 2007-11-06 12:18:14.000-0600 Repository: asterisk Revision: 89037 U branches/1.4/res/res_musiconhold.c ------------------------------------------------------------------------ r89037 | russell | 2007-11-06 12:18:13 -0600 (Tue, 06 Nov 2007) | 11 lines If someone were to delete the files used by an existing MOH class, and then issue a reload, further use of that class could result in a crash due to dividing by zero. This set of changes fixes up some places to prevent this from happening. (closes issue ASTERISK-10500) Reported by: jcomellas Patches: res_musiconhold_division_by_zero.patch uploaded by jcomellas (license 282) Additional changes added by me. ------------------------------------------------------------------------ By: Digium Subversion (svnbot) 2007-11-06 12:21:44.000-0600 Repository: asterisk Revision: 89038 _U trunk/ U trunk/res/res_musiconhold.c ------------------------------------------------------------------------ r89038 | russell | 2007-11-06 12:21:43 -0600 (Tue, 06 Nov 2007) | 19 lines Merged revisions 89037 via svnmerge from https://origsvn.digium.com/svn/asterisk/branches/1.4 ........ r89037 | russell | 2007-11-06 12:20:07 -0600 (Tue, 06 Nov 2007) | 11 lines If someone were to delete the files used by an existing MOH class, and then issue a reload, further use of that class could result in a crash due to dividing by zero. This set of changes fixes up some places to prevent this from happening. (closes issue ASTERISK-10500) Reported by: jcomellas Patches: res_musiconhold_division_by_zero.patch uploaded by jcomellas (license 282) Additional changes added by me. ........ ------------------------------------------------------------------------ By: Digium Subversion (svnbot) 2007-11-06 15:31:34.000-0600 Repository: asterisk Revision: 89066 _U team/murf/fast-ast2/ U team/murf/fast-ast2/apps/app_amd.c U team/murf/fast-ast2/apps/app_chanisavail.c U team/murf/fast-ast2/apps/app_chanspy.c U team/murf/fast-ast2/apps/app_directed_pickup.c U team/murf/fast-ast2/apps/app_exec.c U team/murf/fast-ast2/apps/app_festival.c U team/murf/fast-ast2/apps/app_followme.c U team/murf/fast-ast2/apps/app_forkcdr.c U team/murf/fast-ast2/apps/app_getcpeid.c U team/murf/fast-ast2/apps/app_macro.c U team/murf/fast-ast2/apps/app_minivm.c U team/murf/fast-ast2/apps/app_mixmonitor.c U team/murf/fast-ast2/apps/app_morsecode.c U team/murf/fast-ast2/apps/app_mp3.c U team/murf/fast-ast2/apps/app_nbscat.c U team/murf/fast-ast2/apps/app_playback.c U team/murf/fast-ast2/apps/app_readfile.c U team/murf/fast-ast2/apps/app_sayunixtime.c U team/murf/fast-ast2/apps/app_sms.c U team/murf/fast-ast2/apps/app_softhangup.c U team/murf/fast-ast2/apps/app_speech_utils.c U team/murf/fast-ast2/apps/app_stack.c U team/murf/fast-ast2/apps/app_test.c U team/murf/fast-ast2/apps/app_waitforring.c U team/murf/fast-ast2/apps/app_waitforsilence.c U team/murf/fast-ast2/apps/app_while.c U team/murf/fast-ast2/channels/chan_agent.c U team/murf/fast-ast2/channels/chan_gtalk.c U team/murf/fast-ast2/channels/chan_jingle.c U team/murf/fast-ast2/channels/chan_sip.c U team/murf/fast-ast2/codecs/codec_zap.c U team/murf/fast-ast2/include/asterisk/jabber.h U team/murf/fast-ast2/include/asterisk/lock.h U team/murf/fast-ast2/include/asterisk/tdd.h U team/murf/fast-ast2/main/ast_expr2.fl U team/murf/fast-ast2/main/ast_expr2f.c U team/murf/fast-ast2/main/astmm.c U team/murf/fast-ast2/main/channel.c U team/murf/fast-ast2/main/config.c U team/murf/fast-ast2/main/fskmodem.c U team/murf/fast-ast2/main/loader.c U team/murf/fast-ast2/main/pbx.c U team/murf/fast-ast2/main/tdd.c U team/murf/fast-ast2/res/res_features.c U team/murf/fast-ast2/res/res_indications.c U team/murf/fast-ast2/res/res_jabber.c U team/murf/fast-ast2/res/res_monitor.c U team/murf/fast-ast2/res/res_musiconhold.c ------------------------------------------------------------------------ r89066 | murf | 2007-11-06 15:31:33 -0600 (Tue, 06 Nov 2007) | 188 lines Merged revisions 89031,89034,89038,89041,89043-89044,89047-89052,89054-89055,89057,89062 via svnmerge from https://origsvn.digium.com/svn/asterisk/trunk ................ r89031 | rizzo | 2007-11-06 10:05:13 -0700 (Tue, 06 Nov 2007) | 17 lines Fix embedding of modules on FreeBSD: the constructor for the list of modules was run after the constructors for the embedded modules (which appended entries to the list). As a result, the list appeared empty when it was time to use it. On linux the order of execution of constructor was evidently different (it may depend on the ordering of modules in the ELF file). This is only a workaround - there may be other situations where the execution of constructors causes problems, so if we manage to find a more general solution this workaround can go away. ................ r89034 | file | 2007-11-06 10:10:03 -0700 (Tue, 06 Nov 2007) | 12 lines Merged revisions 89032 via svnmerge from https://origsvn.digium.com/svn/asterisk/branches/1.4 ........ r89032 | file | 2007-11-06 13:08:05 -0400 (Tue, 06 Nov 2007) | 4 lines Make it so that if a peer is determined to be unreachable using qualify their devicestate will report back unavailable. (closes issue ASTERISK-10551) Reported by: pj ........ ................ r89038 | russell | 2007-11-06 11:23:36 -0700 (Tue, 06 Nov 2007) | 19 lines Merged revisions 89037 via svnmerge from https://origsvn.digium.com/svn/asterisk/branches/1.4 ........ r89037 | russell | 2007-11-06 12:20:07 -0600 (Tue, 06 Nov 2007) | 11 lines If someone were to delete the files used by an existing MOH class, and then issue a reload, further use of that class could result in a crash due to dividing by zero. This set of changes fixes up some places to prevent this from happening. (closes issue ASTERISK-10500) Reported by: jcomellas Patches: res_musiconhold_division_by_zero.patch uploaded by jcomellas (license 282) Additional changes added by me. ........ ................ r89041 | qwell | 2007-11-06 11:44:19 -0700 (Tue, 06 Nov 2007) | 4 lines Allow gtalk and jingle to use TLS connections again. Closes issue ASTERISK-9675 ................ r89043 | oej | 2007-11-06 12:04:29 -0700 (Tue, 06 Nov 2007) | 12 lines Merged revisions 89042 via svnmerge from https://origsvn.digium.com/svn/asterisk/branches/1.4 ........ r89042 | oej | 2007-11-06 19:53:37 +0100 (Tis, 06 Nov 2007) | 2 lines Bug fixes to tdd support in zaptel. ........ (Small changes for trunk) ................ r89044 | mmichelson | 2007-11-06 12:04:45 -0700 (Tue, 06 Nov 2007) | 7 lines "show application <foo>" changes for clarity. (closes issue ASTERISK-10696, reported and patched by blitzrage) Many thanks! ................ r89047 | qwell | 2007-11-06 12:10:18 -0700 (Tue, 06 Nov 2007) | 12 lines Merged revisions 89046 via svnmerge from https://origsvn.digium.com/svn/asterisk/branches/1.4 ........ r89046 | qwell | 2007-11-06 13:09:30 -0600 (Tue, 06 Nov 2007) | 4 lines Correctly set the total number of channels from a zaptel transcoder board. SPD-49, patch by Matthew Nicholson. ........ ................ r89048 | oej | 2007-11-06 12:10:26 -0700 (Tue, 06 Nov 2007) | 2 lines Additional TDD changes (preparing for SIP changes - adding TDD support to SIP) ................ r89049 | tilghman | 2007-11-06 12:16:02 -0700 (Tue, 06 Nov 2007) | 10 lines Merged revisions 89045 via svnmerge from https://origsvn.digium.com/svn/asterisk/branches/1.4 ........ r89045 | tilghman | 2007-11-06 13:09:06 -0600 (Tue, 06 Nov 2007) | 2 lines We went to the trouble of creating a method of tracking failed trylocks, then never turned it on (oops). ........ ................ r89050 | oej | 2007-11-06 12:23:10 -0700 (Tue, 06 Nov 2007) | 2 lines Formatting. Illegaly using some spare spaces from Russell's space-bucket. ................ r89051 | murf | 2007-11-06 12:40:33 -0700 (Tue, 06 Nov 2007) | 1 line Hoping to avoid a crash in OSX for a problem blitzrage found ................ r89052 | russell | 2007-11-06 12:51:37 -0700 (Tue, 06 Nov 2007) | 4 lines Fix the memory show allocations CLI command so that it doesn't spew out all of the current memory allocations when you start Asterisk, when the command's handler gets called for initialization. ................ r89054 | russell | 2007-11-06 13:22:50 -0700 (Tue, 06 Nov 2007) | 11 lines Merged revisions 89053 via svnmerge from https://origsvn.digium.com/svn/asterisk/branches/1.4 ........ r89053 | russell | 2007-11-06 14:18:49 -0600 (Tue, 06 Nov 2007) | 3 lines Fix init_classes() so that classes that actually do have files loaded aren't treated as empty, and immediately destroyed ... ........ ................ r89055 | mmichelson | 2007-11-06 13:32:49 -0700 (Tue, 06 Nov 2007) | 9 lines Instead of trying to callback a local channel on a failed attended transfer, call the device that made the transfer instead. This makes for much smoother calling back when queues are involved. (closes issue ASTERISK-10680, reported by IPetrov) Tremendous thanks to Russell for pulling me out of my block I was having on this one ................ r89057 | file | 2007-11-06 13:55:58 -0700 (Tue, 06 Nov 2007) | 4 lines Remove native bridging check for DTMF based transfers. Thanks to the last batch of RTP changes it is no longer required for the media stream to go through Asterisk if DTMF is going over signalling. It will simply reinvite back as needed. (closes issue ASTERISK-10697) Reported by: ibc ................ r89062 | murf | 2007-11-06 14:08:38 -0700 (Tue, 06 Nov 2007) | 9 lines Merged revisions 89036 via svnmerge from https://origsvn.digium.com/svn/asterisk/branches/1.4 ........ r89036 | murf | 2007-11-06 10:52:50 -0700 (Tue, 06 Nov 2007) | 1 line closes issue ASTERISK-8547 - where the [catname](!) and [catname](othercat1,othercat2,...) notation gets dropped across a ConfigUpdate (or any other thing that would cause a config file to be written). While I was at it, I also cleaned up some of the destroy routines to free up comments, which was not being done. Made sure the new struct I introduced is also cleaned up properly at destruction time. My code handles multiple template inclusions. Many thanks to ssokol for his patch, which, while not literally used in the final merge, served as a foundation for the fix. ........ ................ ------------------------------------------------------------------------ By: Digium Subversion (svnbot) 2007-11-09 10:34:33.000-0600 Repository: asterisk Revision: 89131 _U team/file/t38fun/ U team/file/t38fun/apps/app_queue.c U team/file/t38fun/apps/app_voicemail.c U team/file/t38fun/cdr/cdr_tds.c U team/file/t38fun/channels/chan_sip.c U team/file/t38fun/codecs/codec_zap.c U team/file/t38fun/configs/extensions.ael.sample U team/file/t38fun/configs/res_odbc.conf.sample U team/file/t38fun/doc/valgrind.txt U team/file/t38fun/include/asterisk/lock.h U team/file/t38fun/main/config.c U team/file/t38fun/main/manager.c U team/file/t38fun/main/say.c U team/file/t38fun/main/srv.c U team/file/t38fun/main/tdd.c U team/file/t38fun/pbx/pbx_ael.c U team/file/t38fun/res/res_jabber.c U team/file/t38fun/res/res_musiconhold.c ------------------------------------------------------------------------ r89131 | file | 2007-11-09 10:34:31 -0600 (Fri, 09 Nov 2007) | 168 lines Merged revisions 89032,89036-89037,89042,89045-89046,89053,89079,89085,89088,89090,89093,89095,89097,89099,89101,89103,89105,89111,89115,89119,89125 via svnmerge from https://origsvn.digium.com/svn/asterisk/branches/1.4 ........ r89032 | file | 2007-11-06 13:08:05 -0400 (Tue, 06 Nov 2007) | 4 lines Make it so that if a peer is determined to be unreachable using qualify their devicestate will report back unavailable. (closes issue ASTERISK-10551) Reported by: pj ........ r89036 | murf | 2007-11-06 13:52:50 -0400 (Tue, 06 Nov 2007) | 1 line closes issue ASTERISK-8547 - where the [catname](!) and [catname](othercat1,othercat2,...) notation gets dropped across a ConfigUpdate (or any other thing that would cause a config file to be written). While I was at it, I also cleaned up some of the destroy routines to free up comments, which was not being done. Made sure the new struct I introduced is also cleaned up properly at destruction time. My code handles multiple template inclusions. Many thanks to ssokol for his patch, which, while not literally used in the final merge, served as a foundation for the fix. ........ r89037 | russell | 2007-11-06 14:20:07 -0400 (Tue, 06 Nov 2007) | 11 lines If someone were to delete the files used by an existing MOH class, and then issue a reload, further use of that class could result in a crash due to dividing by zero. This set of changes fixes up some places to prevent this from happening. (closes issue ASTERISK-10500) Reported by: jcomellas Patches: res_musiconhold_division_by_zero.patch uploaded by jcomellas (license 282) Additional changes added by me. ........ r89042 | oej | 2007-11-06 14:53:37 -0400 (Tue, 06 Nov 2007) | 2 lines Bug fixes to tdd support in zaptel. ........ r89045 | tilghman | 2007-11-06 15:09:06 -0400 (Tue, 06 Nov 2007) | 2 lines We went to the trouble of creating a method of tracking failed trylocks, then never turned it on (oops). ........ r89046 | qwell | 2007-11-06 15:09:30 -0400 (Tue, 06 Nov 2007) | 4 lines Correctly set the total number of channels from a zaptel transcoder board. SPD-49, patch by Matthew Nicholson. ........ r89053 | russell | 2007-11-06 16:18:49 -0400 (Tue, 06 Nov 2007) | 3 lines Fix init_classes() so that classes that actually do have files loaded aren't treated as empty, and immediately destroyed ... ........ r89079 | tilghman | 2007-11-07 00:07:49 -0400 (Wed, 07 Nov 2007) | 5 lines Suppress AEL warnings on load. Reported by: eliel Patch by: eliel Closes issue ASTERISK-10703 ........ r89085 | mmichelson | 2007-11-07 11:56:49 -0400 (Wed, 07 Nov 2007) | 5 lines Fixing a segfault in the manager "core show channels concise" command. (closes issue ASTERISK-10708, reported by arnd and patched by ys) ........ r89088 | murf | 2007-11-07 17:40:28 -0400 (Wed, 07 Nov 2007) | 1 line In response to 10578, I just ran 1.4 thru valgrind; some of the config leakage I've already fixed, but it doesn't hurt to double check. I found and fixed leaks in res_jabber, cdr_tds, pbx_ael. Nothing major, tho. ........ r89090 | mmichelson | 2007-11-07 18:40:35 -0400 (Wed, 07 Nov 2007) | 6 lines This patch makes it possible for SIP phones to dial extensions defined with '#' characters in extensions.conf AND maintain their escaped characters when forming URI's (closes issue ASTERISK-10265, reported by cahen, patched by me, code review by file) ........ r89093 | tilghman | 2007-11-07 19:39:37 -0400 (Wed, 07 Nov 2007) | 7 lines The member refcount must be incremented, to avoid using it after deallocation. A huge thanks go to lvl- for patiently providing the necessary valgrind output that was necessary to finding this problem of memory corruption. Reported by: lvl- Patch by: tilghman Closes issue ASTERISK-10699 ........ r89095 | file | 2007-11-07 19:53:25 -0400 (Wed, 07 Nov 2007) | 4 lines If callerid is configured in sip.conf use that for checking the presence of an extension in the dialplan. (closes issue ASTERISK-10710) Reported by: spditner ........ r89097 | file | 2007-11-07 21:11:25 -0400 (Wed, 07 Nov 2007) | 8 lines Add support for allowing one outgoing transaction. This means if a response comes back out of order chan_sip will still handle it. I dream of a chan_sip with real transaction support. (closes issue ASTERISK-10498) Reported by: flefoll (closes issue ASTERISK-10472) Reported by: ramonpeek (closes issue ASTERISK-9288) Reported by: atca_pres ........ r89099 | file | 2007-11-07 21:28:56 -0400 (Wed, 07 Nov 2007) | 6 lines Improve the devicestate logic for multiple devices. If any are available then the extension is considered available. (closes issue ASTERISK-9843) Reported by: nic_bellamy Patches: sip-hinting-svn-branch-1.4.patch uploaded by nic (license 299) ........ r89101 | file | 2007-11-07 22:26:48 -0400 (Wed, 07 Nov 2007) | 4 lines Do not add a sip: to the beginning of the To URI unless needed. (closes issue ASTERISK-10331) Reported by: goestelecom ........ r89103 | tilghman | 2007-11-08 00:55:19 -0400 (Thu, 08 Nov 2007) | 2 lines Typo ........ r89105 | kpfleming | 2007-11-08 01:26:47 -0400 (Thu, 08 Nov 2007) | 2 lines fix a glaring bug in the new SRV record handling that would cause incorrect weight sorting ........ r89111 | mmichelson | 2007-11-08 12:47:23 -0400 (Thu, 08 Nov 2007) | 5 lines I made this same adjustment in trunk to fix a bug, and it makes sense to do it in 1.4 as well. If an imapfolder is specified in voicemail.conf, don't ever explicitly connect to INBOX since it may not exist. ........ r89115 | qwell | 2007-11-08 14:45:15 -0400 (Thu, 08 Nov 2007) | 4 lines Avoid warnings on load when using sample configuration files. Issue 11195, patch by eliel. ........ r89119 | mmichelson | 2007-11-08 17:00:08 -0400 (Thu, 08 Nov 2007) | 7 lines Rework of the commit I made yesterday to use the already built-in ast_uri_decode function as opposed to my home-rolled one. Also added comments. Thanks to oej for pointing me in the right direction ........ r89125 | qwell | 2007-11-08 19:52:35 -0400 (Thu, 08 Nov 2007) | 4 lines Properly say the seconds here.. Issue 11203, fix described by vma. ........ ------------------------------------------------------------------------ |