packaging
/
ostree
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

lib/repo: cleanup_tmpdir should be executed after releasing lock file 

Here's a subtle bug in abort_transaction():
One of the policies of cleaning up is to skip the current boot's staging
directory. The responsible function for this is cleanup_tmpdir() which tries
to lock each of the tmpdir before deleting it. When it comes to the current
boot's staging dir, it tries to lock the directory(again!) but fails as there
is already a lockfile present. Just because the current boot's staging dir was
meant to be skipped, the bug never surfaced up and wasn't catastrohpic.

if (!_ostree_repo_try_lock_tmpdir (dfd, path, &lockfile, &did_lock, error))
  return FALSE;
if (!did_lock)
  return TRUE; /* Note early return */
...
if (g_str_has_prefix (path, self->stagedir_prefix))
  return TRUE; /* Note early return */

The actual check for skipping staging dir for current boot was never reached
because the function returned at did_lock failure.

Therefore, execute cleanup_tmpdir() after releasing the lockfile in
abort_transaction() so that cleanup_tmpdir gets a chance to lock current boot's
staging directory and succeed.

Closes: #1602
Approved by: jlebon
This commit is contained in:
Umang Jain 2018-06-27 02:41:43 +05:30 committed by Atomic Bot
parent 9f48e212a3
commit 1074668ede
1 changed files with 4 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
src/libostree/ostree-repo-commit.c
View File

@ -2229,10 +2229,6 @@ ostree_repo_abort_transaction (OstreeRepo *self,
if (!self->in_transaction)
return TRUE;
/* Do not propagate failures from cleanup_tmpdir() immediately, as we want
* to clean up the rest of the internal transaction state first. */
cleanup_tmpdir (self, cancellable, &cleanup_error);
if (self->loose_object_devino_hash)
g_hash_table_remove_all (self->loose_object_devino_hash);
@ -2242,6 +2238,10 @@ ostree_repo_abort_transaction (OstreeRepo *self,
glnx_tmpdir_unset (&self->commit_stagedir);
glnx_release_lock_file (&self->commit_stagedir_lock);
/* Do not propagate failures from cleanup_tmpdir() immediately, as we want
* to clean up the rest of the internal transaction state first. */
cleanup_tmpdir (self, cancellable, &cleanup_error);
self->in_transaction = FALSE;
if (self->txn_locked)