Skip to content

Commit

Permalink
repo: Make locking precondition failures fatal
Browse files Browse the repository at this point in the history
Use `g_error` and `g_assert*` rather than `g_return*` when checking the
locking preconditions so that failures result in the program
terminating. Since this code is protecting filesystem data, we'd rather
crash than delete or corrupt data unexpectedly.

`g_error` is used when the error is due to the caller requesting an
invalid transition like attempting to pop a lock type that hasn't been
taken. It also provides a semi-useful message about what happened.

(cherry picked from commit 89f4ce2)
  • Loading branch information
dbnicholson committed Jun 7, 2021
1 parent 4204a7b commit 46e74d3
Showing 1 changed file with 11 additions and 5 deletions.
16 changes: 11 additions & 5 deletions src/libostree/ostree-repo.c
Original file line number Diff line number Diff line change
Expand Up @@ -354,7 +354,8 @@ push_repo_lock (OstreeRepo *self,
counter = &(self->lock.shared);

/* Check for overflow */
g_assert_cmpuint (*counter, <, G_MAXUINT);
if (*counter == G_MAXUINT)
g_error ("Repo lock %s counter would overflow", lock_state_name (next_state));

if (info.state == LOCK_EX || info.state == next_state)
{
Expand Down Expand Up @@ -387,13 +388,16 @@ pop_repo_lock (OstreeRepo *self,
int flags = blocking ? 0 : LOCK_NB;

g_autoptr(GMutexLocker) locker = g_mutex_locker_new (&self->lock.mutex);
g_return_val_if_fail (self->lock.fd != -1, FALSE);
if (self->lock.fd == -1)
g_error ("Cannot pop repo never locked repo lock");

OstreeRepoLockInfo info;
repo_lock_info (self, locker, &info);
g_return_val_if_fail (info.len > 0, FALSE);
g_debug ("Pop lock: state=%s, depth=%u", info.name, info.len);

if (info.len == 0 || info.state == LOCK_UN)
g_error ("Cannot pop already unlocked repo lock");

int state_to_drop;
guint *counter;
if (lock_type == OSTREE_REPO_LOCK_EXCLUSIVE)
Expand All @@ -408,7 +412,9 @@ pop_repo_lock (OstreeRepo *self,
}

/* Make sure caller specified a valid type to release */
g_assert_cmpuint (*counter, >, 0);
if (*counter == 0)
g_error ("Repo %s lock pop requested, but none have been taken",
lock_state_name (state_to_drop));

int next_state;
if (info.len == 1)
Expand All @@ -434,7 +440,7 @@ pop_repo_lock (OstreeRepo *self,
else
{
/* We should never drop from shared to exclusive */
g_return_val_if_fail (next_state == LOCK_SH, FALSE);
g_assert (next_state == LOCK_SH);
g_debug ("Returning lock state to shared");
if (!do_repo_lock (self->lock.fd, next_state | flags))
return glnx_throw_errno_prefix (error,
Expand Down

0 comments on commit 46e74d3

Please sign in to comment.