Browse Source

storage: avoid concurrent umask()

umask() modifies the whole process, which could lead to odd permission
problems in concurrent code. Use explicit fchmod() calls instead.

Preserve the legacy storage/file behavior of creating files 0o664 in
FileVolume.create() and 0o644 (from the normal qubesd umask) otherwise.
Rusty Bird 3 years ago
parent
commit
d4b1794c15
2 changed files with 4 additions and 4 deletions
  1. 0 2
      qubes/storage/__init__.py
  2. 4 2
      qubes/storage/file.py

+ 0 - 2
qubes/storage/__init__.py

@@ -540,10 +540,8 @@ class Storage:
     @asyncio.coroutine
     def create(self):
         ''' Creates volumes on disk '''
-        old_umask = os.umask(0o002)
         yield from qubes.utils.void_coros_maybe(
             vol.create() for vol in self.vm.volumes.values())
-        os.umask(old_umask)
 
     @asyncio.coroutine
     def clone_volume(self, src_vm, name):

+ 4 - 2
qubes/storage/file.py

@@ -211,7 +211,7 @@ class FileVolume(qubes.storage.Volume):
         assert isinstance(self.size, int) and self.size > 0, \
             'Volume size must be > 0'
         if not self.snap_on_start:
-            create_sparse_file(self.path, self.size)
+            create_sparse_file(self.path, self.size, permissions=0o664)
 
     def remove(self):
         if not self.snap_on_start:
@@ -455,7 +455,7 @@ class FileVolume(qubes.storage.Volume):
 
 
 
-def create_sparse_file(path, size):
+def create_sparse_file(path, size, permissions=None):
     ''' Create an empty sparse file '''
     if os.path.exists(path):
         raise IOError("Volume %s already exists" % path)
@@ -463,6 +463,8 @@ def create_sparse_file(path, size):
     if not os.path.exists(parent_dir):
         os.makedirs(parent_dir)
     with open(path, 'a+b') as fh:
+        if permissions is not None:
+            os.fchmod(fh.fileno(), permissions)
         fh.truncate(size)