From 89f4ce2c1d3cacccee8129ac54bd60775dbbe5d2 Mon Sep 17 00:00:00 2001 From: Dan Nicholson Date: Thu, 6 May 2021 16:49:51 -0600 Subject: [PATCH] repo: Make locking precondition failures fatal 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. --- src/libostree/ostree-repo.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/src/libostree/ostree-repo.c b/src/libostree/ostree-repo.c index 73f374e5..8fe3812d 100644 --- a/src/libostree/ostree-repo.c +++ b/src/libostree/ostree-repo.c @@ -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) { @@ -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) @@ -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) @@ -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,