shaba/systemd
1
0
Fork 0
mirror of https://github.com/systemd/systemd.git synced 2025-03-19 22:50:17 +03:00
Code

varlink: avoid using dangling ref in varlink_close_unref()

Browse Source 
Fixes #18025, https://bugzilla.redhat.com/show_bug.cgi?id=1931034.

We drop the reference stored in Manager.managed_oom_varlink_request in two code paths:
vl_disconnect() which is installed as a disconnect callback, and in manager_varlink_done().
But we also make a disconnect from manager_varlink_done(). So we end up with the following
call stack:

(gdb) bt
 0  vl_disconnect (s=0x112c7b0, link=0xea0070, userdata=0xe9bcc0) at ../src/core/core-varlink.c:414
 1  0x00007f1366e9d5ac in varlink_detach_server (v=0xea0070) at ../src/shared/varlink.c:1210
 2  0x00007f1366e9d664 in varlink_close (v=0xea0070) at ../src/shared/varlink.c:1228
 3  0x00007f1366e9d6b5 in varlink_close_unref (v=0xea0070) at ../src/shared/varlink.c:1240
 4  0x0000000000524629 in manager_varlink_done (m=0xe9bcc0) at ../src/core/core-varlink.c:479
 5  0x000000000048ef7b in manager_free (m=0xe9bcc0) at ../src/core/manager.c:1357
 6  0x000000000042602c in main (argc=5, argv=0x7fff439c43d8) at ../src/core/main.c:2909

When we enter vl_disconnect(), m->managed_oom_varlink_request.n_ref==1.
When we exit from vl_discconect(), m->managed_oom_varlink_request==NULL. But
varlink_close_unref() has a copy of the pointer in *v. When we continue executing
varlink_close_unref(), this pointer is dangling, and the call to varlink_unref()
is done with an invalid pointer.
This commit is contained in:
Zbigniew Jędrzejewski-Szmek 2021-03-07 16:42:35 +01:00
parent 8b0f54c929
commit 39ad3f1c09
1 changed files with 25 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

33
src/shared/varlink.c
View File

@ -1206,8 +1206,9 @@ int varlink_close(Varlink *v) {
varlink_set_state(v, VARLINK_DISCONNECTED);
/* Let's take a reference first, since varlink_detach_server() might drop the final (dangling) ref
* which would destroy us before we can call varlink_clear() */
/* Let's take a reference first, since varlink_detach_server() might drop the final ref from the
* disconnect callback, which would invalidate the pointer we are holding before we can call
* varlink_clear(). */
varlink_ref(v);
varlink_detach_server(v);
varlink_clear(v);
@ -1220,17 +1221,33 @@ Varlink* varlink_close_unref(Varlink *v) {
if (!v)
return NULL;
(void) varlink_close(v);
/* A reference is given to us to be destroyed. But when calling varlink_close(), a callback might
* also drop a reference. We allow this, and will hold a temporary reference to the object to make
* sure that the object still exists when control returns to us. If there's just one reference
* remaining after varlink_close(), even though there were at least two right before, we'll handle
* that gracefully instead of crashing.
*
* In other words, this call drops the donated reference, but if the internal call to varlink_close()
* dropped a reference to, we don't drop the reference afain. This allows the caller to say:
* global_object->varlink = varlink_close_unref(global_object->varlink);
* even though there is some callback which has access to global_object and may drop the reference
* stored in global_object->varlink. Without this step, the same code would have to be written as:
* Varlink *t = TAKE_PTR(global_object->varlink);
* varlink_close_unref(t);
*/
/* n_ref >= 1 */
varlink_ref(v); /* n_ref >= 2 */
varlink_close(v); /* n_ref >= 1 */
if (v->n_ref > 1)
v->n_ref--; /* n_ref >= 1 */
return varlink_unref(v);
}
Varlink* varlink_flush_close_unref(Varlink *v) {
if (!v)
return NULL;
if (v)
varlink_flush(v);
(void) varlink_flush(v);
(void) varlink_close(v);
return varlink_unref(v);
return varlink_close_unref(v);
}
static int varlink_enqueue_json(Varlink *v, JsonVariant *m) {