From: David Herrmann Date: Thu, 8 Jan 2015 15:57:30 +0000 (+0100) Subject: reply: fix ref-leak X-Git-Tag: upstream/0.20150129.081441utc~67 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=b6bcf921d865515151ac389d579640b4139244eb;p=platform%2Fcore%2Fsystem%2Fkdbus-bus.git reply: fix ref-leak Introduce kdbus_reply_link/unlink() which take care of the list-owned reference to the reply object. Fix all the callsides to use it and properly let each caller own its own reference now. We no longer have to be aware of the queue-state at all times, instead, the list-state owns its own reference. Signed-off-by: David Herrmann --- diff --git a/connection.c b/connection.c index 4c7d3de..19a41dc 100644 --- a/connection.c +++ b/connection.c @@ -165,17 +165,15 @@ int kdbus_cmd_msg_recv(struct kdbus_conn *conn, */ mutex_lock(&reply->reply_dst->lock); if (!list_empty(&reply->entry)) { - if (reply->sync) { + kdbus_reply_unlink(reply); + if (reply->sync) kdbus_sync_reply_wakeup(reply, -EPIPE); - } else { - list_del_init(&reply->entry); + else kdbus_notify_reply_dead(conn->ep->bus, entry->msg.src_id, entry->msg.cookie); - } } mutex_unlock(&reply->reply_dst->lock); - kdbus_reply_unref(reply); } kdbus_notify_flush(conn->ep->bus); @@ -255,11 +253,9 @@ static int kdbus_conn_check_access(struct kdbus_conn *conn_src, mutex_lock(&conn_dst->lock); r = kdbus_reply_find(conn_src, conn_dst, msg->cookie_reply); if (r) { - list_del_init(&r->entry); if (r->sync) *reply_wake = kdbus_reply_ref(r); - else - kdbus_reply_unref(r); + kdbus_reply_unlink(r); } mutex_unlock(&conn_dst->lock); @@ -341,8 +337,8 @@ static int kdbus_conn_entry_sync_attach(struct kdbus_conn *conn_dst, remote_ret = -EREMOTEIO; kdbus_sync_reply_wakeup(reply_wake, remote_ret); + kdbus_reply_unlink(reply_wake); mutex_unlock(&reply_wake->reply_dst->lock); - kdbus_reply_unref(reply_wake); return ret; } @@ -400,7 +396,7 @@ int kdbus_conn_entry_insert(struct kdbus_conn *conn_src, entry->reply = kdbus_reply_ref(reply); if (reply) { - list_add(&reply->entry, &conn_src->reply_list); + kdbus_reply_link(reply); if (!reply->sync) schedule_delayed_work(&conn_src->work, 0); } @@ -541,7 +537,6 @@ static int kdbus_conn_wait_reply(struct kdbus_conn *conn_src, } mutex_lock(&conn_src->lock); - list_del_init(&reply_wait->entry); reply_wait->waiting = false; entry = reply_wait->queue_entry; if (entry) { @@ -553,6 +548,7 @@ static int kdbus_conn_wait_reply(struct kdbus_conn *conn_src, kdbus_pool_slice_release(entry->slice); kdbus_queue_entry_free(entry); } + kdbus_reply_unlink(reply_wait); mutex_unlock(&conn_src->lock); return ret; @@ -708,10 +704,12 @@ int kdbus_cmd_msg_send(struct kdbus_conn *conn_src, reply_wait = kdbus_reply_find(conn_dst, conn_src, kmsg->msg.cookie); if (reply_wait) { - if (reply_wait->interrupted) + if (reply_wait->interrupted) { + kdbus_reply_ref(reply_wait); reply_wait->interrupted = false; - else + } else { reply_wait = NULL; + } } mutex_unlock(&conn_src->lock); @@ -827,6 +825,7 @@ wait_sync: exit_unref: kdbus_reply_unref(reply_wait); + kdbus_reply_unref(reply_wake); kdbus_conn_unref(conn_dst); exit_name_unlock: kdbus_name_unlock(bus->name_registry, name_entry); @@ -926,10 +925,8 @@ int kdbus_conn_disconnect(struct kdbus_conn *conn, bool ensure_queue_empty) kdbus_queue_entry_free(entry); } - list_for_each_entry_safe(r, r_tmp, &conn->reply_list, entry) { - list_del_init(&r->entry); - kdbus_reply_unref(r); - } + list_for_each_entry_safe(r, r_tmp, &conn->reply_list, entry) + kdbus_reply_unlink(r); mutex_unlock(&conn->lock); /* lock order: domain -> bus -> ep -> names -> conn */ @@ -940,14 +937,13 @@ int kdbus_conn_disconnect(struct kdbus_conn *conn, bool ensure_queue_empty) if (r->reply_src == conn) { if (r->sync) { kdbus_sync_reply_wakeup(r, -EPIPE); + kdbus_reply_unlink(r); continue; } /* send a 'connection dead' notification */ kdbus_notify_reply_dead(bus, c->id, r->cookie); - - list_del_init(&r->entry); - kdbus_reply_unref(r); + kdbus_reply_unlink(r); } } mutex_unlock(&c->lock); diff --git a/queue.c b/queue.c index cfaae5e..8f36ddd 100644 --- a/queue.c +++ b/queue.c @@ -35,6 +35,7 @@ #include "message.h" #include "metadata.h" #include "queue.h" +#include "reply.h" /** * kdbus_queue_entry_add() - Add an queue entry to a queue @@ -483,6 +484,7 @@ void kdbus_queue_entry_free(struct kdbus_queue_entry *entry) { kdbus_msg_resources_unref(entry->msg_res); kdbus_meta_unref(entry->meta); + kdbus_reply_unref(entry->reply); kfree(entry->msg_extra); kfree(entry); } diff --git a/reply.c b/reply.c index 6186eb5..d979a0b 100644 --- a/reply.c +++ b/reply.c @@ -51,6 +51,7 @@ struct kdbus_reply *kdbus_reply_new(struct kdbus_conn *reply_src, } kref_init(&r->kref); + INIT_LIST_HEAD(&r->entry); r->reply_src = kdbus_conn_ref(reply_src); r->reply_dst = kdbus_conn_ref(reply_dst); r->cookie = msg->cookie; @@ -108,6 +109,31 @@ struct kdbus_reply *kdbus_reply_unref(struct kdbus_reply *r) return NULL; } +/** + * kdbus_reply_link() - Link reply object into target connection + * @r: Reply to link + */ +void kdbus_reply_link(struct kdbus_reply *r) +{ + if (WARN_ON(!list_empty(&r->entry))) + return; + + list_add(&r->entry, &r->reply_dst->reply_list); + kdbus_reply_ref(r); +} + +/** + * kdbus_reply_unlink() - Unlink reply object from target connection + * @r: Reply to unlink + */ +void kdbus_reply_unlink(struct kdbus_reply *r) +{ + if (!list_empty(&r->entry)) { + list_del_init(&r->entry); + kdbus_reply_unref(r); + } +} + /** * kdbus_sync_reply_wakeup() - Wake a synchronously blocking reply * @reply: The reply object @@ -122,7 +148,6 @@ void kdbus_sync_reply_wakeup(struct kdbus_reply *reply, int err) if (WARN_ON(!reply->sync)) return; - list_del_init(&reply->entry); reply->waiting = false; reply->err = err; wake_up_interruptible(&reply->reply_dst->wait); @@ -217,8 +242,7 @@ void kdbus_reply_list_scan(struct kdbus_conn *conn) kdbus_notify_reply_timeout(conn->ep->bus, conn->id, reply->cookie); - list_del_init(&reply->entry); - kdbus_reply_unref(reply); + kdbus_reply_unlink(reply); } /* rearm delayed work with next timeout */ diff --git a/reply.h b/reply.h index 7d003ef..832fccb 100644 --- a/reply.h +++ b/reply.h @@ -55,6 +55,9 @@ struct kdbus_reply *kdbus_reply_new(struct kdbus_conn *reply_src, struct kdbus_reply *kdbus_reply_ref(struct kdbus_reply *r); struct kdbus_reply *kdbus_reply_unref(struct kdbus_reply *r); +void kdbus_reply_link(struct kdbus_reply *r); +void kdbus_reply_unlink(struct kdbus_reply *r); + struct kdbus_reply *kdbus_reply_find(struct kdbus_conn *replying, struct kdbus_conn *reply_dst, u64 cookie);