From: Greg Hudson <ghudson@mit.edu>
Date: Sat, 14 Apr 2018 18:43:22 +0000 (-0400)
Subject: Avoid simultaneous KDB/ulog locks in ulog_replay
X-Git-Tag: krb5-1.17-beta1~132
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=682013f55984173f2f742c0ecf43d83181100456;p=thirdparty%2Fkrb5.git

Avoid simultaneous KDB/ulog locks in ulog_replay

In ulog_replay(), instead of locking the KDB and ulog for the whole
operation, separately lock and update the ulog after each database
update, as we would for a locally initiated database operation (after
commit 444ef5fe9ec8d64a5db27b3a8aaf6813dd7ef0e0).

ticket: 8664 (new)
---

diff --git a/src/lib/kdb/kdb_log.c b/src/lib/kdb/kdb_log.c
index 766d3002a3..e48f59a881 100644
--- a/src/lib/kdb/kdb_log.c
+++ b/src/lib/kdb/kdb_log.c
@@ -369,19 +369,10 @@ ulog_replay(krb5_context context, kdb_incr_result_t *incr_ret, char **db_args)
 
     INIT_ULOG(context);
 
-    /* Lock the DB before the ulog to avoid deadlock. */
     retval = krb5_db_open(context, db_args,
                           KRB5_KDB_OPEN_RW | KRB5_KDB_SRV_TYPE_ADMIN);
     if (retval)
         return retval;
-    retval = krb5_db_lock(context, KRB5_DB_LOCKMODE_EXCLUSIVE);
-    if (retval)
-        return retval;
-    retval = lock_ulog(context, KRB5_LOCKMODE_EXCLUSIVE);
-    if (retval) {
-        krb5_db_unlock(context);
-        return retval;
-    }
 
     no_of_updates = incr_ret->updates.kdb_ulog_t_len;
     upd = incr_ret->updates.kdb_ulog_t_val;
@@ -391,11 +382,7 @@ ulog_replay(krb5_context context, kdb_incr_result_t *incr_ret, char **db_args)
         if (!upd->kdb_commit)
             continue;
 
-        /* If (unexpectedly) this update does not follow the last one we
-         * stored, discard any previous ulog state. */
-        if (ulog->kdb_num != 0 && upd->kdb_entry_sno != ulog->kdb_last_sno + 1)
-            reset_ulog(log_ctx);
-
+        /* Replay this update in the database. */
         if (upd->kdb_deleted) {
             dbprincstr = k5memdup0(upd->kdb_princ_name.utf8str_t_val,
                                    upd->kdb_princ_name.utf8str_t_len, &retval);
@@ -424,7 +411,18 @@ ulog_replay(krb5_context context, kdb_incr_result_t *incr_ret, char **db_args)
                 goto cleanup;
         }
 
+        retval = lock_ulog(context, KRB5_LOCKMODE_EXCLUSIVE);
+        if (retval)
+            goto cleanup;
+
+        /* If (unexpectedly) this update does not follow the last one we
+         * stored, discard any previous ulog state. */
+        if (ulog->kdb_num != 0 && upd->kdb_entry_sno != ulog->kdb_last_sno + 1)
+            reset_ulog(log_ctx);
+
+        /* Store this update in the ulog for any downstream KDCs. */
         retval = store_update(log_ctx, upd);
+        unlock_ulog(context);
         if (retval)
             goto cleanup;
 
@@ -432,12 +430,10 @@ ulog_replay(krb5_context context, kdb_incr_result_t *incr_ret, char **db_args)
     }
 
 cleanup:
+    if (retval)
+        (void)ulog_init_header(context);
     if (fupd)
         ulog_free_entries(fupd, no_of_updates);
-    if (retval)
-        reset_ulog(log_ctx);
-    unlock_ulog(context);
-    krb5_db_unlock(context);
     return retval;
 }