repo: Make locking precondition failures fatal
authorDan Nicholson <dbn@endlessos.org>
Thu, 6 May 2021 22:49:51 +0000 (16:49 -0600)
committerDan Nicholson <dbn@endlessos.org>
Sat, 5 Jun 2021 15:15:34 +0000 (09:15 -0600)
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

index 73f374e5f2d0a3e6063543c2f7de92ac4b368eb4..8fe3812dc0e2fb5784054c62b3f70b7acde34727 100644 (file)
@@ -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,