Qubes/core-admin
1
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

core: fix race condition in qubes.xml locking (#906)

Browse Source 
QubesVmCollection.save() overrides qubes.xml by creating new file, then
renaming it over the old one. If any process has that (old) file open
at the same time - especially while waiting on lock_db_for_writing() -
it will end up in accessing old, already unlinked file.

The exact calls would look like:
P1                                      P2
lock_db_for_writing
  fd = open('qubes.xml')
  fcntl(fd, F_SETLK, ...)

                                      lock_db_for_writing
                                          fd = open('qubes.xml')
                                          fcntl(fd, F_SETLK, ...)
...
save():
    open(temp-file)
    write(temp-file, ...)
    ...
    flush(temp-file)
    rename(temp-file, 'qubes.xml')
    close(fd) // close old file

                                      lock_db_for_writing succeed
                                      *** fd points at already unlinked
                                          file
unlock_db
    close(qubes.xml)

To fix that problem, added a check if (already locked) file is still the
same as qubes.xml.
This commit is contained in:
Marek Marczykowski-Górecki 2014-10-22 03:53:30 +02:00
parent 9324cdb175
commit 247cff335f
1 changed files with 20 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

24
core/qubes.py
View File

@ -526,12 +526,28 @@ class QubesVmCollection(dict):
self.save() self.save()
def lock_db_for_reading(self): def lock_db_for_reading(self):
self.qubes_store_file = open (self.qubes_store_filename, 'r') # save() would rename the file over qubes.xml, _then_ release lock,
fcntl.lockf (self.qubes_store_file, fcntl.LOCK_SH) # so we need to ensure that the file for which we've got the lock is
# still the right file
while True:
self.qubes_store_file = open (self.qubes_store_filename, 'r')
fcntl.lockf(self.qubes_store_file, fcntl.LOCK_SH)
if os.fstat(self.qubes_store_file.fileno()) == os.stat(
self.qubes_store_filename):
break
self.qubes_store_file.close()
def lock_db_for_writing(self): def lock_db_for_writing(self):
self.qubes_store_file = open (self.qubes_store_filename, 'r+') # save() would rename the file over qubes.xml, _then_ release lock,
fcntl.lockf (self.qubes_store_file, fcntl.LOCK_EX) # so we need to ensure that the file for which we've got the lock is
# still the right file
while True:
self.qubes_store_file = open (self.qubes_store_filename, 'r+')
fcntl.lockf(self.qubes_store_file, fcntl.LOCK_EX)
if os.fstat(self.qubes_store_file.fileno()) == os.stat(
self.qubes_store_filename):
break
self.qubes_store_file.close()
def unlock_db(self): def unlock_db(self):
# intentionally do not call explicit unlock to not unlock the file # intentionally do not call explicit unlock to not unlock the file