From: David Herrmann <dh.herrmann@gmail.com>
Date: Thu, 23 Oct 2014 11:30:13 +0000 (+0200)
Subject: connection: dont leak replies if message-queueing fails
X-Git-Tag: upstream/0.20141102.012929utc~38
X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=fbe016ac90cbbbd6385283effe7150711221926c;p=platform%2Fcore%2Fsystem%2Fkdbus-bus.git

connection: dont leak replies if message-queueing fails

Currently, we queue the conn_reply _before_ queueing the actual message.
This might leak conn_reply objects if we cannot queue the message. Avoid
this by queuing the conn_reply object at the same time we queue the
message.

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

diff --git a/connection.c b/connection.c
index 7dd4229..9d48cc3 100644
--- a/connection.c
+++ b/connection.c
@@ -402,12 +402,6 @@ static int kdbus_conn_add_expected_reply(struct kdbus_conn *conn_src,
 		goto exit_dec_reply_count;
 	}
 
-	mutex_lock(&conn_dst->lock);
-	if (!kdbus_conn_active(conn_dst)) {
-		ret = -ECONNRESET;
-		goto exit_unlock;
-	}
-
 	/*
 	 * This message expects a reply, so let's interpret msg->timeout_ns and
 	 * add a kdbus_conn_reply object. Add it to the list of expected replies
@@ -418,7 +412,7 @@ static int kdbus_conn_add_expected_reply(struct kdbus_conn *conn_src,
 	r = kzalloc(sizeof(*r), GFP_KERNEL);
 	if (!r) {
 		ret = -ENOMEM;
-		goto exit_unlock;
+		goto exit_dec_reply_count;
 	}
 
 	r->reply_dst = kdbus_conn_ref(conn_src);
@@ -432,22 +426,8 @@ static int kdbus_conn_add_expected_reply(struct kdbus_conn *conn_src,
 		r->deadline_ns = msg->timeout_ns;
 	}
 
-	list_add(&r->entry, &conn_dst->reply_list);
 	*reply_wait = r;
 
-	/*
-	 * For async operation, schedule the scan now. It won't do any real work
-	 * at this point, but walk the list of all pending replies and rearm the
-	 * connection's delayed work to the closest entry.
-	 * For synchronous operation, the timeout will be handled by
-	 * wait_event_interruptible_timeout().
-	 */
-	if (!sync)
-		schedule_delayed_work(&conn_dst->work, 0);
-
-exit_unlock:
-	mutex_unlock(&conn_dst->lock);
-
 exit_dec_reply_count:
 	if (ret < 0)
 		atomic_dec(&conn_src->reply_count);
@@ -500,6 +480,12 @@ static int kdbus_conn_entry_insert(struct kdbus_conn *conn,
 	 */
 	entry->reply = reply;
 
+	if (reply) {
+		list_add(&reply->entry, &conn->reply_list);
+		if (!reply->sync)
+			schedule_delayed_work(&conn->work, 0);
+	}
+
 	/* link the message into the receiver's entry */
 	kdbus_queue_entry_add(&conn->queue, entry);
 	mutex_unlock(&conn->lock);
@@ -787,6 +773,10 @@ int kdbus_conn_kmsg_send(struct kdbus_ep *ep,
 		kmsg->dst_name_id = name_entry->name_id;
 
 	if (conn_src) {
+		ret = kdbus_kmsg_attach_metadata(kmsg, conn_src, conn_dst);
+		if (ret < 0)
+			goto exit_unref;
+
 		if (msg->flags & KDBUS_MSG_FLAGS_EXPECT_REPLY) {
 			ret = kdbus_conn_check_access(ep, msg, conn_src,
 						      conn_dst, NULL);
@@ -804,10 +794,6 @@ int kdbus_conn_kmsg_send(struct kdbus_ep *ep,
 			if (ret < 0)
 				goto exit_unref;
 		}
-
-		ret = kdbus_kmsg_attach_metadata(kmsg, conn_src, conn_dst);
-		if (ret < 0)
-			goto exit_unref;
 	}
 
 	if (reply_wake) {
@@ -825,6 +811,9 @@ int kdbus_conn_kmsg_send(struct kdbus_ep *ep,
 
 		kdbus_conn_reply_sync(reply_wake, ret);
 		mutex_unlock(&conn_dst->lock);
+
+		if (ret < 0)
+			goto exit_unref;
 	} else {
 		/*
 		 * Otherwise, put it in the queue and wait for the connection
@@ -832,11 +821,13 @@ int kdbus_conn_kmsg_send(struct kdbus_ep *ep,
 		 */
 		ret = kdbus_conn_entry_insert(conn_dst, conn_src,
 					      kmsg, reply_wait);
+		if (ret < 0) {
+			if (reply_wait)
+				kdbus_conn_reply_free(reply_wait);
+			goto exit_unref;
+		}
 	}
 
-	if (ret < 0)
-		goto exit_unref;
-
 	/* forward to monitors */
 	kdbus_conn_eavesdrop(ep, conn_src, kmsg);