summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorDavid Herrmann <dh.herrmann@gmail.com>2014-03-22 17:43:30 +0100
committerDavid Herrmann <dh.herrmann@gmail.com>2014-03-22 18:00:03 +0100
commit22fdeadcc06e95fe41ac4de872ec245c0887547f (patch)
treefe8387b2605767f4b20d36158ad7d6d64aa472d6
parent2afa65c3122542b21d1881fada917c89b8913bd4 (diff)
sd-rtnl: fix self-reference leaks
Like sd-bus, sd-rtnl can have self-references through queued messages. In particular, each queued message has the following self-ref loop: rtnl->wqueue[i]->rtnl == rtnl Same is true for "rqueue". When sd_rtnl_unref() gets called, we must therefore make sure we correctly consider each self-reference when deciding to destroy the object. For each queued message, there _might_ be one ref. However, rtnl-messages can be created _without_ a bus-reference, therefore we need to verify the actually required ref-count. Once we know exactly how many self-refs exist, and we verified none of the queued messages has external references, we can destruct the object. We must immediately drop our own reference, then flush all queues and destroy the bus object. Otherwise, each sd_rtnl_message_unref() call would recurse into the same destruction logic as they enter with the same rtnl-refcnt. Note: We really should verify _all_ queued messages have m->rtnl set to the bus they're queued on. If that's given, we can change: if (REFCNT_GET(rtnl->n_ref) <= refs) to if (REFCNT_GET(rtnl->n_ref) == refs) and thus avoid recalculating the required refs for each message we remove from the queue during destruction.
-rw-r--r--src/libsystemd/sd-rtnl/sd-rtnl.c81
1 files changed, 66 insertions, 15 deletions
diff --git a/src/libsystemd/sd-rtnl/sd-rtnl.c b/src/libsystemd/sd-rtnl/sd-rtnl.c
index e5610b4335..6042657642 100644
--- a/src/libsystemd/sd-rtnl/sd-rtnl.c
+++ b/src/libsystemd/sd-rtnl/sd-rtnl.c
@@ -111,31 +111,82 @@ sd_rtnl *sd_rtnl_ref(sd_rtnl *rtnl) {
}
sd_rtnl *sd_rtnl_unref(sd_rtnl *rtnl) {
+ unsigned long refs;
- if (rtnl && REFCNT_DEC(rtnl->n_ref) <= 0) {
+ if (!rtnl)
+ return NULL;
+
+ /*
+ * If our ref-cnt is exactly the number of internally queued messages
+ * plus the ref-cnt to be dropped, then we know there's no external
+ * reference to us. Hence, we look through all queued messages and if
+ * they also have no external references, we're about to drop the last
+ * ref. Flush the queues so the REFCNT_DEC() below will drop to 0.
+ * We must be careful not to introduce inter-message references or this
+ * logic will fall apart..
+ */
+
+ refs = rtnl->rqueue_size + rtnl->wqueue_size + 1;
+
+ if (REFCNT_GET(rtnl->n_ref) <= refs) {
struct match_callback *f;
+ bool q = true;
unsigned i;
- for (i = 0; i < rtnl->rqueue_size; i++)
- sd_rtnl_message_unref(rtnl->rqueue[i]);
- free(rtnl->rqueue);
+ for (i = 0; i < rtnl->rqueue_size; i++) {
+ if (REFCNT_GET(rtnl->rqueue[i]->n_ref) > 1) {
+ q = false;
+ break;
+ } else if (rtnl->rqueue[i]->rtnl != rtnl)
+ --refs;
+ }
- for (i = 0; i < rtnl->wqueue_size; i++)
- sd_rtnl_message_unref(rtnl->wqueue[i]);
- free(rtnl->wqueue);
+ if (q) {
+ for (i = 0; i < rtnl->wqueue_size; i++) {
+ if (REFCNT_GET(rtnl->wqueue[i]->n_ref) > 1) {
+ q = false;
+ break;
+ } else if (rtnl->wqueue[i]->rtnl != rtnl)
+ --refs;
+ }
+ }
- hashmap_free_free(rtnl->reply_callbacks);
- prioq_free(rtnl->reply_callbacks_prioq);
+ if (q && REFCNT_GET(rtnl->n_ref) == refs) {
+ /* Drop our own ref early to avoid recursion from:
+ * sd_rtnl_message_unref()
+ * sd_rtnl_unref()
+ * These must enter sd_rtnl_unref() with a ref-cnt
+ * smaller than us. */
+ REFCNT_DEC(rtnl->n_ref);
- while ((f = rtnl->match_callbacks)) {
- LIST_REMOVE(match_callbacks, rtnl->match_callbacks, f);
- free(f);
- }
+ for (i = 0; i < rtnl->rqueue_size; i++)
+ sd_rtnl_message_unref(rtnl->rqueue[i]);
+ free(rtnl->rqueue);
- safe_close(rtnl->fd);
- free(rtnl);
+ for (i = 0; i < rtnl->wqueue_size; i++)
+ sd_rtnl_message_unref(rtnl->wqueue[i]);
+ free(rtnl->wqueue);
+
+ assert_se(REFCNT_GET(rtnl->n_ref) == 0);
+
+ hashmap_free_free(rtnl->reply_callbacks);
+ prioq_free(rtnl->reply_callbacks_prioq);
+
+ while ((f = rtnl->match_callbacks)) {
+ LIST_REMOVE(match_callbacks, rtnl->match_callbacks, f);
+ free(f);
+ }
+
+ safe_close(rtnl->fd);
+ free(rtnl);
+
+ return NULL;
+ }
}
+ assert_se(REFCNT_GET(rtnl->n_ref) > 0);
+ REFCNT_DEC(rtnl->n_ref);
+
return NULL;
}