[Home]

Summary:ASTERISK-13966: voicemail umask / permissions bug
Reporter:Jim Capp (jcapp)Labels:
Date Opened:2009-04-16 09:44:36Date Closed:2009-04-16 16:06:20
Priority:MajorRegression?No
Status:Closed/CompleteComponents: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.