Bug 60338 - FILESAVE: Saved files have incorrect permissions on linux
Summary: FILESAVE: Saved files have incorrect permissions on linux
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: UI (show other bugs)
Version:
(earliest affected)
3.6.5.2 release
Hardware: Other Linux (All)
: medium normal
Assignee: Stephan Bergmann
URL:
Whiteboard: BSA Confirmed:4.2.0.1:Ubuntu target:4...
Keywords:
Depends on:
Blocks:
 
Reported: 2013-02-05 22:25 UTC by dvcroft
Modified: 2014-05-27 08:22 UTC (History)
3 users (show)

See Also:
Crash report or crash signature:


Attachments
A sample file with a macro that will save 10000 identical pdfs. Run this test case and look for incorrect permissions on the saved pdfs. (31.55 KB, application/vnd.oasis.opendocument.text)
2013-02-05 22:25 UTC, dvcroft
Details

Note You need to log in before you can comment on or make changes to this bug.
Description dvcroft 2013-02-05 22:25:44 UTC
Created attachment 74259 [details]
A sample file with a macro that will save 10000 identical pdfs. Run this test case and look for incorrect permissions on the saved pdfs.

Problem description: 
On our office machines we have umask set to 002. Normally files saved from LibreOffice will be saved with permissions rw-rw-r.

With no discernible cause, LibreOffice will suddenly start saving files with permissions rw------.

We have duplicated this problem on nfs shares and on a local drive. The problem comes and goes. We have seen it primarily on exported pdf's that we create with a macro, but we have also seen it on calc documents that are saved manually, and a few times on writer documents. Once the problem starts it seems to affect other documents being saved, until the problem ends. Closing and restarting LibreOffice corrects the problem temporarily.


Steps to reproduce:
This is a very random problem. Saving 10s of files manually I did not get an occurrence. I created a macro to save 10000 files. Even then I do not always see the problem. You may have to follow the steps below multiple times to see the problem.
0. Type umask from a command prompt and make sure you have a umask that allows group and world permissions. (002 or 022 are common). Save a file from LibreOffice and check the permissions to see that there are some permissions shown for group or world.
1. Open the attached document and enable macros to run.
2. In the main macro of the document, edit the path variable to a directory on your local machine where you can have 10000 new files created.
3. Run the macro
4. While the macro is running, edit and save documents in other LibreOffice windows and even use different programs on your computer. (This seems to increase the chances of seeing the problem).
5. When the macro has finished running, try the following command from the directory where the files were saved:
  find -perm 600
If any files are returned, you have duplicated the problem.
3. Type
    ls -lt | less
   and look at the permissions and see how they change over time



Current behavior:
Seemingly randomly, saved files will have only rw------ permissions, meaning no one but the owner can read (or write) them.

Expected behavior:
With a umask of 002, saved files should have permissions rw-rw-r and they usually do.

Operating System: Linux (Other)
Version: 3.6.5.2 release
Comment 1 dvcroft 2013-02-21 16:36:38 UTC
I recognize this is a strange bug that is not going to get a lot of attention. I'm willing to try and develop a fix myself, but am completely unfamiliar with the LibreOffice code base. Can anyone give me some general direction as to where the code for writing a pdf would be located.
Comment 2 Robinson Tryon (qubit) 2013-12-25 17:52:50 UTC
TESTING on Ubuntu 12.04.3 with
LibreOffice Version: 4.2.0.1

(In reply to comment #1)
> I recognize this is a strange bug that is not going to get a lot of
> attention.

Indeed -- I did try to repro, but did not manage to do so... :P

> I'm willing to try and develop a fix myself, but am completely
> unfamiliar with the LibreOffice code base. Can anyone give me some general
> direction as to where the code for writing a pdf would be located.

There are some notes about the location of the PDF export code here:
http://lists.freedesktop.org/archives/libreoffice/2013-September/055693.html

Feel free to ask questions on the #libreoffice-dev IRC channel as well!
Comment 3 Robinson Tryon (qubit) 2013-12-25 17:58:29 UTC
(In reply to comment #2)
> TESTING on Ubuntu 12.04.3 with
> LibreOffice Version: 4.2.0.1
> 
> (In reply to comment #1)
> > I recognize this is a strange bug that is not going to get a lot of
> > attention.
> 
> Indeed -- I did try to repro, but did not manage to do so... :P

Aaaaaaaand I speak too soon... :P

It must be a Christmas miracle! I was just able to reproduce on my 2nd run. PDFs from 4946 and on were all turning up with '600' perms.

Status -> NEW

Good luck tracking this problem down!
Comment 4 dvcroft 2014-01-07 00:26:54 UTC
Thanks for looking at this and confirming it.

In my original description I mentioned we had seen the problem a few times with writer and calc files. Since then we have seen it a lot more in those native formats.

Because of that, I believe the problem is in some kind of low level file routine that is used by multiple formats. Probably caused by some kind of uninitialized memory or variable. Any thoughts about where I could look for some shared file system code?

Thanks!
Comment 5 jwoithe 2014-03-13 04:57:43 UTC
A user of mine reported a very similar problem to this earlier today.  This was the libreoffice.org 32-bit binary release of 4.2.1 running under 32-bit Linux (Slackware 13.37).  They took an existing ODT file created libreoffice 4.0.2 (let's call it A.odt) and did File|SaveAs to create a fresh copy under a different name (B.odt).  The newly created ODT file (B.odt) had permissions 600, as oppposed to 644 that has been seen when the exact same process was done in the past.

Furthermore, when the newly created file (B.odt) was File|ExportToPDF, the resulting PDF file had 600 permissions.  Changing the ODT to have 644 using chmod, loading the result and exporting to PDF still had the PDF with 644 permissions.

Loading A.odt and exporting to PDF had 600 permissions on the PDF.

Creating a new file and saving that gave the new file 644 permissions, as expected.  Loading this file and doing "SaveAs" resulted in 644 permissions on the new ODT file.  Exporting either file resulted in permissions of 644 on the PDF files.  After doing this, loading B.odt and repeating the export to PDF resulted in 600 permissions on the PDF.

I then loaded another ODT file (C.odt) which had been created with an earlier version of libreoffice.  Exporting this to PDF resulted in 600 permissions.

At this stage it seemed the bogus permissions when B.odt was exported to PDF were 100% reproducible.  Unfortunately it is not possible to post that file due to the nature of the information it contains.  I then attempted to delete things from it, testing the permissions of the exported PDF at various times.  I got nearly to the top of the file when the PDF suddenly started acquiring 644 permissions.  I reloaded C.odt, did an export, and got 600 permissions.  I returned to B.odt, inserted a space, resaved, exported and had 600 permissions.  I removed single block of text, exported, and got 644 permissions.  I went back to B.pdf and exported without making any changes, and got 644.  From this point on, no matter what I did I was not able to get anything except the expected 644 permissions.

While it initially appeared that the documents we had exhibited the problem on demand, the issue mysteriously went away without anything being done beyond the repeated libreoffice tests as described.  The exact same procedure which triggered the problem earlier every time it was done now completely fails to do the wrong thing.  Nothing changed on the computer concerned and there were no logout/login sequences in between.  I have noticed that unlike 4.1.x and earlier, with version 4.2.1 I do now see "XDM authorization key matches an existing client!" each time I start libreoffice, but I very much doubt this has anything to do with the permission problems described, especially since this message still appears when the permission malfunction does not.

Note that we ourselves have not observed this problem under any earlier version of libreoffice.

Based on the above descriptions, it seems that libreoffice 4.2.1 can be added to the versions which suffer from the problem.  Furthermore, it appears that dvcroft's suggestion of an uninitialised variable (or something similar) is quite likely given the intermittant nature of the misbehaviour.
Comment 6 lj618 2014-03-28 15:35:41 UTC
(In reply to comment #5)
> ...
> I have
> noticed that unlike 4.1.x and earlier, with version 4.2.1 I do now see "XDM
> authorization key matches an existing client!" each time I start
> libreoffice, but I very much doubt this has anything to do with the
> permission problems described...

I just wrote up bug 76742 on this problem (also on Slackware). A work-around is included. I also do not believe it is related to the permissions problem.
Comment 7 ulkitz 2014-05-04 20:59:51 UTC
The described behauviour of the attached document might be caused by codes like this (from tempfile.c):

mode_t old_mode = umask(077);
	    
            osl_error = osl_openFile(
                tmp_file_url,
                file_handle,
                osl_File_OpenFlag_Read |
                osl_File_OpenFlag_Write |
                osl_File_OpenFlag_Create);

            umask(old_mode)

Thread1 sets the umask to 077 and stores the actual umask in old_mode, then control switches to another thread which as well stores the actual umask (now 077), then thread1 restores the umask to old_mode, then thread2 restores it to 077, and the original umask is gone forever.

I normally lost umask around file 1200, but after synchronizing the calls to umask I succedded to export ~3500 pdfs with rw-r--r-- (I set umask to 022), so there might be other similar code fragments.
Comment 8 dvcroft 2014-05-05 17:06:08 UTC
(In reply to comment #7)
> The described behauviour of the attached document might be caused by codes
> like this (from tempfile.c):
> 
> mode_t old_mode = umask(077);
> 	    
>             osl_error = osl_openFile(
>                 tmp_file_url,
>                 file_handle,
>                 osl_File_OpenFlag_Read |
>                 osl_File_OpenFlag_Write |
>                 osl_File_OpenFlag_Create);
> 
>             umask(old_mode)
> 
> Thread1 sets the umask to 077 and stores the actual umask in old_mode, then
> control switches to another thread which as well stores the actual umask
> (now 077), then thread1 restores the umask to old_mode, then thread2
> restores it to 077, and the original umask is gone forever.
> 
> I normally lost umask around file 1200, but after synchronizing the calls to
> umask I succedded to export ~3500 pdfs with rw-r--r-- (I set umask to 022),
> so there might be other similar code fragments.

That could definitely cause the problem we're seeing, thanks for your work on this!
Comment 9 Stephan Bergmann 2014-05-12 14:30:44 UTC
Ulrich, thanks a lot for your analysis and patch <https://gerrit.libreoffice.org/#/c/9287/1>.  Looks indeed very plausible that the uncoordinated calls to umask(3) are causing this.

However, I would prefer to rather get rid of those umask(3) calls.  (That way, for one, unrelated open(3) calls are not accidentally affected by umask modifications, and , for another, we likely do not need to introduce new URE API.)

Looking at the existing umask(3) calls, there is at least three different use cases:

1  Inside the sal/osl/unx implementation of osl_createTempFile.  This can be fixed internally in sal/osl.

2  In the implementation of utl::TempFile.  This can hopefully be addressed by basing the required functionality on osl_createTempFile.

3  In places where Coverity warned about uses of mkstemp "without securely setting umask first."  I think that Coverity warning is bogus (cf. <https://communities.coverity.com/message/6516> "Why are uses of mkstemp 'without securely setting umask first' being flagged?") and those calls to umask should be reverted again
Comment 10 Commit Notification 2014-05-13 09:41:18 UTC
Stephan Bergmann committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=8fd54c107a01f98cf9c731104b0d3eaa2b85bfa4

Related fdo#60338: Do not use umask(3) in a MT program



The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 11 Commit Notification 2014-05-13 10:29:37 UTC
Stephan Bergmann committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=229e85acd222a4ae9647c7a970269c87cb875b40

elated fdo#60338: #error on umask(3) calls in currently unused code



The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 12 Commit Notification 2014-05-13 10:42:51 UTC
Stephan Bergmann committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=d62b6d845c75e9c6074d821b425035fbee9c1559

Related fdo#60338: do not call umask(3) in a MT program



The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 13 Commit Notification 2014-05-13 12:17:03 UTC
Stephan Bergmann committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=533237fec4b91fb5f871e0b5028586516dd8c0be

Related fdo#60338: Restrictive open mode flags for tempfile w/o calling umask



The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 14 Commit Notification 2014-05-13 12:32:23 UTC
Stephan Bergmann committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=034d4c111976f337877e5954c6cc95ed9f90a343

Related fdo#60338: Setting umask for osl::Directory::open is useless



The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 15 Commit Notification 2014-05-13 12:37:12 UTC
Stephan Bergmann committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=1d72a0262c4570631d0aa8f98e34e21fb9d6ae42

Related fdo#60338: Create missing temp file dir with user's original umask



The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 16 dvcroft 2014-05-13 15:16:21 UTC
Thanks for all your work Stephan. I like your solution of leaving the umask alone!
Comment 17 jwoithe 2014-05-14 00:18:27 UTC
This sounds promising.  I will test the daily build in a day or so (allowing time for the fix to propagate there).

The whiteboard indicates the fix is targetting 4.3.0.  Since this problem more or less makes 4.2.x a non-starter in a business environment (where preservation of access modes is important), is there any chance it could be pushed into a 4.2.x maintenance release as well?  It appears to be a fairly important bugfix to me, at least as far as Linux users are concerned.
Comment 18 Stephan Bergmann 2014-05-14 07:11:53 UTC
(In reply to comment #17)
> This sounds promising.  I will test the daily build in a day or so (allowing
> time for the fix to propagate there).

Note that the issue is not fully fixed yet.  I'm pushing incremental changes to master, and some robot is setting this issue's whiteboard target when it sees commits that mention this issue's ID.  I will make note here once the issue is fully fixed on master.

> The whiteboard indicates the fix is targetting 4.3.0.  Since this problem
> more or less makes 4.2.x a non-starter in a business environment (where
> preservation of access modes is important), is there any chance it could be
> pushed into a 4.2.x maintenance release as well?  It appears to be a fairly
> important bugfix to me, at least as far as Linux users are concerned.

Depending on the ultimate fix, backporting could be somewhat tricky.  And given that this issue is reported against LO 3.6 (and is probably even older than LO itself), I'm not sure how pressing a fix for 4.2 vs. 4.3 actually is overall.
Comment 19 jwoithe 2014-05-14 08:15:35 UTC
(In reply to comment #18)
> Note that the issue is not fully fixed yet.  I'm pushing incremental changes
> to master, and some robot is setting this issue's whiteboard target when it
> sees commits that mention this issue's ID.  I will make note here once the
> issue is fully fixed on master.

Ok, no worries.

> Depending on the ultimate fix, backporting could be somewhat tricky.  And
> given that this issue is reported against LO 3.6 (and is probably even older
> than LO itself), I'm not sure how pressing a fix for 4.2 vs. 4.3 actually is
> overall.

Obviously I can only speak from what I have experienced on systems here.  For us this issue appeared only in LO 4.2.x.  We have not observed it in earlier versions.  Given what we now know about the problem it seems possible that the problem was, by luck, simply much less likely to be triggered in earlier versions.  However, this appears to have changed with 4.2.x whereas at least on our systems the umask race is now very frequently triggered.  This of course could be due to any number of things since it would ultimately be a timing thing between threads, and it may even be that our systems are the only ones on which the fault is triggered so quickly under 4.2.x.

I understand the potential difficulties in backporting a fix especially if it is invasive.  In any case, until it is fixed we are pretty much stuck on 4.1.x.

I will keen an eye out for your "fully fixed" notification here as I am keen to test the daily build to confirm the fix on our systems.  Thanks for your efforts to fix this issue.
Comment 20 Commit Notification 2014-05-19 09:45:19 UTC
Stephan Bergmann committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=f67702e709a9049042fdd456d2456d35055a935d

fdo#60338: Add osl_File_OpenFlag_Private to avoid umask



The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 21 Commit Notification 2014-05-20 16:15:00 UTC
Stephan Bergmann committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=f65de4feee506b80be0986ce28f7a2311e4fbe2c

fdo#60338: Introduce osl_createDirectoryWithFlags



The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 22 jwoithe 2014-05-27 00:35:33 UTC
I would like to test the daily build to confirm that the permissions problem is fixed on our systems.  The automated commit messages say that the changes have been pushed to "master".  Looking at the daily build directories I see items for "libreoffice-4-3" and "master".  At this stage (and taking into account comment 18) I would assume that the fix is only in "master" and has not yet been sent into 4.3.  This would make the "master" daily build the one to test - right?

What is the expected version that this fix will land in - comment 18 indicated that we won't see it in 4.2, but is there a chance it'll make it into 4.3?
Comment 23 Stephan Bergmann 2014-05-27 08:22:54 UTC
(In reply to comment #22)
> What is the expected version that this fix will land in - comment 18
> indicated that we won't see it in 4.2, but is there a chance it'll make it
> into 4.3?

All the above fixes for this issue went into master before the 4.3 branch-off, so the issue should be fully fixed in current 4.3 beta builds.