krb5 commit: Avoid simultaneous KDB/ulog locks in ulog_replay

[Greg Hudson]

https://github.com/krb5/krb5/commit/682013f55984173f2f742c0ecf43d83181100456
commit 682013f55984173f2f742c0ecf43d83181100456
Author: Greg Hudson <ghudson at mit.edu>
Date:   Sat Apr 14 14:43:22 2018 -0400

    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)

 src/lib/kdb/kdb_log.c |   32 ++++++++++++++------------------
 1 files changed, 14 insertions(+), 18 deletions(-)

diff --git a/src/lib/kdb/kdb_log.c b/src/lib/kdb/kdb_log.c
index 766d300..e48f59a 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;
 }