File 0055-vici-dont-lock-connection-in-write-mode-when-enabling-on_write-callback.patch of Package strongswan.28851

commit 2cb6d144a6f3da6d1f006bf8252fa90a9514d435
Author: Tobias Brunner <tobias@strongswan.org>
Date:   Thu Apr 13 13:55:00 2023 +0200

    vici: Don't lock connection in write mode when enabling on_write() callback
    
    This should prevent a deadlock that could previously be caused when a
    control-log event was raised.  The deadlock looked something like this:
    
     * Thread A holds the read lock on bus_t and raises the control-log event.
       This requires acquiring the connection entry in write mode to queue the
       outgoing message.  If it is already held by another thread, this blocks
       on a condvar.
    
     * Thread B is registering the on_write() callback on the same connection's
       stream due to a previous log message.  Before this change, the code
       acquired the entry in write mode as well, thus, blocking thread A.  To
       remove/add the stream, the mutex in watcher_t needs to be acquired.
    
     * Thread C is in watcher_t's watch() and holds the mutex while logging on
       level 2 or 3.  The latter requires the read lock on bus_t, which should
       usually be acquirable even if thread A holds it.  Unless writers are
       concurrently waiting on the lock and the implementation is blocking
       new readers to prevent writer starvation.
    
     * Thread D is removing a logger from the bus (e.g. after an initiate()
       call) and is waiting to acquire the write lock on bus_t and is thereby
       blocking thread C.
    
    With this change, thread B should not block thread A anymore.  So thread D
    and thread C should eventually be able to proceed as well.
    
    Thread A could be held up a bit if there is a thread already sending
    messages for the same connection, but that should only cause a delay, no
    deadlock, as on_write() and do_write() don't log (or lock) anything while
    keeping the entry locked in write mode.
    
    Closes strongswan/strongswan#566

diff --git a/src/libcharon/plugins/vici/vici_socket.c b/src/libcharon/plugins/vici/vici_socket.c
index 21bdead14..39d34e4d3 100644
--- a/src/libcharon/plugins/vici/vici_socket.c
+++ b/src/libcharon/plugins/vici/vici_socket.c
@@ -127,6 +127,8 @@ typedef struct {
 	int readers;
 	/** any users writing over this connection? */
 	int writers;
+	/** any users using this connection at all? */
+	int users;
 	/** condvar to wait for usage  */
 	condvar_t *cond;
 } entry_t;
@@ -211,6 +213,7 @@ static entry_t* find_entry(private_vici_socket_t *this, stream_t *stream,
 			{
 				entry->writers++;
 			}
+			entry->users++;
 			found = entry;
 			break;
 		}
@@ -240,7 +243,7 @@ static entry_t* remove_entry(private_vici_socket_t *this, u_int id)
 			if (entry->id == id)
 			{
 				candidate = TRUE;
-				if (entry->readers || entry->writers)
+				if (entry->readers || entry->writers || entry->users)
 				{
 					entry->cond->wait(entry->cond, this->mutex);
 					break;
@@ -273,6 +276,7 @@ static void put_entry(private_vici_socket_t *this, entry_t *entry,
 	{
 		entry->writers--;
 	}
+	entry->users--;
 	entry->cond->signal(entry->cond);
 	this->mutex->unlock(this->mutex);
 }
@@ -583,6 +587,7 @@ CALLBACK(on_accept, bool,
 		.queue = array_create(sizeof(chunk_t), 0),
 		.cond = condvar_create(CONDVAR_TYPE_DEFAULT),
 		.readers = 1,
+		.users = 1,
 	);
 
 	this->mutex->lock(this->mutex);
@@ -606,11 +611,13 @@ CALLBACK(enable_writer, job_requeue_t,
 {
 	entry_t *entry;
 
-	entry = find_entry(sel->this, NULL, sel->id, FALSE, TRUE);
+	/* we don't modify the in- or outbound queue, so don't lock the entry in
+	 * reader or writer mode */
+	entry = find_entry(sel->this, NULL, sel->id, FALSE, FALSE);
 	if (entry)
 	{
 		entry->stream->on_write(entry->stream, on_write, sel->this);
-		put_entry(sel->this, entry, FALSE, TRUE);
+		put_entry(sel->this, entry, FALSE, FALSE);
 	}
 	return JOB_REQUEUE_NONE;
 }