Summary: | ASTERISK-13966: voicemail umask / permissions bug | ||
Reporter: | Jim Capp (jcapp) | Labels: | |
Date Opened: | 2009-04-16 09:44:36 | Date Closed: | 2009-04-16 16:06:20 |
Priority: | Major | Regression? | No |
Status: | Closed/Complete | Components: | Applications/app_voicemail |
Versions: | Frequency of Occurrence | ||
Related Issues: | |||
Environment: | Attachments: | ||
Description: | The voicemail application has a bug where, depending on how it is compiled, it doesn't retain the umask value that is read upon module load. This is the real source of the problem related to issues 0009231 and 0007504. The problem is that since my_umask is not declared static, compilers are not required to persist its value. As such, some highly optimized compilers will assign it a cpu register rather than variable space on the stack. Since my_umask is only set during module load, subsequent uses of my_umask are unreliable as the register will surely have been overwritten prior to its next use. What made this difficult to diagnose was that the values seemed reasonably close to real umask values, as the functions prior to its use were related to the loading of the mask constant VOICEMAIL_FILE_MODE 0666. The solution is simply to make my_umask static in app_voicemail.c From what I can tell, this applies to ALL versions of app_voicemail.c including the latest 1.4 and 1.6 branches. | ||
Comments: | By: Leif Madsen (lmadsen) 2009-04-16 11:03:28 From the asterisk-dev mailing list (Kevin P. Fleming): This is incorrect. Since 'my_umask' is a module level variable (file scope) its value *must* be preserved across function calls, since all functions within that module share that variable. In addition, since it is not static, then it becomes an exported symbol from that module and could be accessed from other modules (compilation units). Without the compiler being told otherwise, it *must* treat the variable this way, including placing the symbol for this variable into the run-time symbol table for the module for the linker to be able to resolve references to at runtime. With all of this, I do not see how it could ever be placed into a register, and that's not even taking into account the fact that the variable can be accessed from multiple functions in app_voicemail that can be called from various different call paths, and it would not be possible to ensure that the register that had been assigned was preserved through all those code paths. Now, in spite of all of that, making this variable static *is* the right thing to do, since it does not need to be exported (in fact, for nearly all modules in Asterisk, file-scope variables should *always* be static). If this fixes the bug that's great, but I without more evidence I don't see how the reason why it fixes the bug is what you have described. By: Tilghman Lesher (tilghman) 2009-04-16 14:58:47 In the future, please do not place patches in notes, but instead upload them to the file upload area after signing a license agreement. By: Jim Capp (jcapp) 2009-04-16 15:08:00 By the way, this is not so much a bug in Asterisk, as it is venturing on compiler activity that is undefined in an edge case. Making the variable static removes any doubt as to how the compiler must handle the variable. I've already signed the license agreement. If the patch wasn't so trivial, I would have uploaded a file. By: Digium Subversion (svnbot) 2009-04-16 16:02:30 Repository: asterisk Revision: 188773 U branches/1.4/apps/app_voicemail.c ------------------------------------------------------------------------ r188773 | tilghman | 2009-04-16 16:02:29 -0500 (Thu, 16 Apr 2009) | 4 lines Umask should not be exported into global namespace. (closes issue ASTERISK-13966) Reported by: jcapp ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=188773 By: Digium Subversion (svnbot) 2009-04-16 16:03:32 Repository: asterisk Revision: 188774 _U trunk/ U trunk/apps/app_voicemail.c ------------------------------------------------------------------------ r188774 | tilghman | 2009-04-16 16:03:32 -0500 (Thu, 16 Apr 2009) | 11 lines Merged revisions 188773 via svnmerge from https://origsvn.digium.com/svn/asterisk/branches/1.4 ........ r188773 | tilghman | 2009-04-16 16:02:29 -0500 (Thu, 16 Apr 2009) | 4 lines Umask should not be exported into global namespace. (closes issue ASTERISK-13966) Reported by: jcapp ........ ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=188774 By: Digium Subversion (svnbot) 2009-04-16 16:04:53 Repository: asterisk Revision: 188775 _U branches/1.6.0/ U branches/1.6.0/apps/app_voicemail.c ------------------------------------------------------------------------ r188775 | tilghman | 2009-04-16 16:04:53 -0500 (Thu, 16 Apr 2009) | 18 lines Merged revisions 188774 via svnmerge from https://origsvn.digium.com/svn/asterisk/trunk ................ r188774 | tilghman | 2009-04-16 16:03:31 -0500 (Thu, 16 Apr 2009) | 11 lines Merged revisions 188773 via svnmerge from https://origsvn.digium.com/svn/asterisk/branches/1.4 ........ r188773 | tilghman | 2009-04-16 16:02:29 -0500 (Thu, 16 Apr 2009) | 4 lines Umask should not be exported into global namespace. (closes issue ASTERISK-13966) Reported by: jcapp ........ ................ ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=188775 By: Digium Subversion (svnbot) 2009-04-16 16:05:35 Repository: asterisk Revision: 188776 _U branches/1.6.1/ U branches/1.6.1/apps/app_voicemail.c ------------------------------------------------------------------------ r188776 | tilghman | 2009-04-16 16:05:35 -0500 (Thu, 16 Apr 2009) | 18 lines Merged revisions 188774 via svnmerge from https://origsvn.digium.com/svn/asterisk/trunk ................ r188774 | tilghman | 2009-04-16 16:03:31 -0500 (Thu, 16 Apr 2009) | 11 lines Merged revisions 188773 via svnmerge from https://origsvn.digium.com/svn/asterisk/branches/1.4 ........ r188773 | tilghman | 2009-04-16 16:02:29 -0500 (Thu, 16 Apr 2009) | 4 lines Umask should not be exported into global namespace. (closes issue ASTERISK-13966) Reported by: jcapp ........ ................ ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=188776 By: Digium Subversion (svnbot) 2009-04-16 16:06:20 Repository: asterisk Revision: 188777 _U branches/1.6.2/ U branches/1.6.2/apps/app_voicemail.c ------------------------------------------------------------------------ r188777 | tilghman | 2009-04-16 16:06:19 -0500 (Thu, 16 Apr 2009) | 18 lines Merged revisions 188774 via svnmerge from https://origsvn.digium.com/svn/asterisk/trunk ................ r188774 | tilghman | 2009-04-16 16:03:31 -0500 (Thu, 16 Apr 2009) | 11 lines Merged revisions 188773 via svnmerge from https://origsvn.digium.com/svn/asterisk/branches/1.4 ........ r188773 | tilghman | 2009-04-16 16:02:29 -0500 (Thu, 16 Apr 2009) | 4 lines Umask should not be exported into global namespace. (closes issue ASTERISK-13966) Reported by: jcapp ........ ................ ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=188777 By: Sverre G (sverre) 2016-10-11 23:36:21.123-0500 I spent ages on the problem in which voicemail messages had permissions I didn't like and came across this thread once I started digging around in the voicemail.c code. What I discovered is that ~my_umask contains the umask that the asterisk process was started with (often 0644), and when joined with VOICEMAIL_FILE_MODE (0666) the result is the more restrictive mode (0644). The solution is to modify /usr/bin/safe_asterisk, specifically the line #UMASK=022, uncomment it and change it to UMASK=000 or UMASK=011. This will result in ~my_umask having the value 0777 or 0766, which when merged with 0666 results in 0666 and thus all voicemail files will have permissions of -rw-rw-rw which is much more useful (particularly if you have some third party web based system for listening to and deleting voicemails). I hope this helps someone so far down the track. |