connection: simplify reply cleanups

There is no reason why we cannot destroy replies while holding a
connection lock. If the reply points to the connection whose lock we hold,
we also have another ref on that connection due to our context. If the
lock points to another connection, we can simply unref it at any time.

Note that we never cause disconnects on the connection. We only unref it!
The object destruction is a simple memory cleanup. Nothing fancy is done
there, and no inter-object refs can exist anymore (otherwise, it would not
get freed). Therefore, fix all our callers to free replies directly,
instead of releasing the locks first.

Signed-off-by: David Herrmann <dh.herrmann@gmail.com>

diff --git a/connection.c b/connection.c
index d40b6ae6bc49df490febfe94f06787f48a9b11f6..52c4d9b76b179e78f962d6261806551f766e3507 100644 (file)
--- a/connection.c
+++ b/connection.c
@@ -144,7 +144,6 @@ static void kdbus_conn_work(struct work_struct *work)
 {
        struct kdbus_conn *conn;
        struct kdbus_conn_reply *reply, *reply_tmp;
-       LIST_HEAD(reply_list);
        u64 deadline = ~0ULL;
        struct timespec64 ts;
        u64 now;
@@ -176,22 +175,16 @@ static void kdbus_conn_work(struct work_struct *work)
                        continue;
                }
 
-               /*
-                * Move to temporary cleanup list; we cannot unref and
-                * possibly cleanup a connection that is holding a ref
-                * back to us, while we are locking ourselves.
-                */
-               list_move_tail(&reply->entry, &reply_list);
-
                /*
                 * A zero deadline means the connection died, was
                 * cleaned up already and the notification was sent.
                 */
-               if (reply->deadline_ns == 0)
-                       continue;
+               if (reply->deadline_ns != 0)
+                       kdbus_notify_reply_timeout(conn->bus, reply->conn->id,
+                                                  reply->cookie);
 
-               kdbus_notify_reply_timeout(conn->bus, reply->conn->id,
-                                          reply->cookie);
+               list_del_init(&reply->entry);
+               kdbus_conn_reply_free(reply);
        }
 
        /* rearm delayed work with next timeout */
@@ -202,9 +195,6 @@ static void kdbus_conn_work(struct work_struct *work)
        mutex_unlock(&conn->lock);
 
        kdbus_notify_flush(conn->bus);
-
-       list_for_each_entry_safe(reply, reply_tmp, &reply_list, entry)
-               kdbus_conn_reply_free(reply);
 }
 
 /**
@@ -234,7 +224,6 @@ int kdbus_cmd_msg_recv(struct kdbus_conn *conn,
 
        /* just drop the message */
        if (recv->flags & KDBUS_RECV_DROP) {
-               struct kdbus_conn_reply *reply = NULL;
                bool reply_found = false;
 
                if (entry->reply) {
@@ -261,7 +250,7 @@ int kdbus_cmd_msg_recv(struct kdbus_conn *conn,
                                kdbus_conn_reply_sync(entry->reply, -EPIPE);
                        } else {
                                list_del_init(&entry->reply->entry);
-                               reply = entry->reply;
+                               kdbus_conn_reply_free(entry->reply);
                        }
 
                        kdbus_notify_reply_dead(conn->bus,
@@ -273,9 +262,6 @@ int kdbus_cmd_msg_recv(struct kdbus_conn *conn,
                kdbus_pool_slice_free(entry->slice);
                mutex_unlock(&conn->lock);
 
-               if (reply)
-                       kdbus_conn_reply_free(reply);
-
                kdbus_queue_entry_free(entry);
 
                goto exit;
@@ -368,25 +354,23 @@ static int kdbus_conn_check_access(struct kdbus_ep *ep,
         */
        if (reply_wake && msg->cookie_reply > 0) {
                struct kdbus_conn_reply *r, *tmp;
-               LIST_HEAD(reply_list);
 
                mutex_lock(&conn_src->lock);
                list_for_each_entry_safe(r, tmp, &conn_src->reply_list, entry) {
                        if (r->conn == conn_dst &&
                            r->cookie == msg->cookie_reply) {
-                               if (r->sync)
+                               if (r->sync) {
                                        *reply_wake = r;
-                               else
-                                       list_move_tail(&r->entry, &reply_list);
+                               } else {
+                                       list_del_init(&r->entry);
+                                       kdbus_conn_reply_free(r);
+                               }
 
                                allowed = true;
                                break;
                        }
                }
                mutex_unlock(&conn_src->lock);
-
-               list_for_each_entry_safe(r, tmp, &reply_list, entry)
-                       kdbus_conn_reply_free(r);
        }
 
        if (allowed)