From dc4783d6c8512a2e78f32c1452632e055666dc76 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 6 Jun 2018 11:12:25 +0200 Subject: shared/bus-util: use the new cleanup functionality to avoid a memleak MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If the async callbacks didn't get a chance to finish properly, we'd leak memory. The output from test-bus-util with additional log line in the callbacks to show what is happening: $ build/test-bus-util /* test_name_async (0) */ Bus test-bus: changing state UNSET → OPENING Bus test-bus: changing state OPENING → AUTHENTICATING Bus test-bus: changing state AUTHENTICATING → HELLO Sent message type=method_call sender=n/a destination=org.freedesktop.DBus path=/org/freedesktop/DBus interface=org.freedesktop.DBus member=Hello cookie=1 reply_cookie=0 signature=n/a error-name=n/a error-message=n/a Sent message type=method_call sender=n/a destination=org.freedesktop.DBus path=/org/freedesktop/DBus interface=org.freedesktop.DBus member=RequestName cookie=2 reply_cookie=0 signature=su error-name=n/a error-message=n/a Got message type=method_return sender=org.freedesktop.DBus destination=:1.732 path=n/a interface=n/a member=n/a cookie=4294967295 reply_cookie=1 signature=s error-name=n/a error-message=n/a Bus test-bus: changing state HELLO → RUNNING Bus test-bus: changing state RUNNING → CLOSED request_name_destroy_callback n_ref=1 /* test_name_async (20) */ Bus test-bus: changing state UNSET → OPENING Bus test-bus: changing state OPENING → AUTHENTICATING Bus test-bus: changing state AUTHENTICATING → HELLO stage 0: sd_bus_process returned 1 Sent message type=method_call sender=n/a destination=org.freedesktop.DBus path=/org/freedesktop/DBus interface=org.freedesktop.DBus member=Hello cookie=1 reply_cookie=0 signature=n/a error-name=n/a error-message=n/a Sent message type=method_call sender=n/a destination=org.freedesktop.DBus path=/org/freedesktop/DBus interface=org.freedesktop.DBus member=RequestName cookie=2 reply_cookie=0 signature=su error-name=n/a error-message=n/a stage 1: sd_bus_process returned 1 Got message type=method_return sender=org.freedesktop.DBus destination=:1.733 path=n/a interface=n/a member=n/a cookie=4294967295 reply_cookie=1 signature=s error-name=n/a error-message=n/a Bus test-bus: changing state HELLO → RUNNING stage 2: sd_bus_process returned 1 Got message type=signal sender=org.freedesktop.DBus.Local destination=n/a path=/org/freedesktop/DBus/Local interface=org.freedesktop.DBus.Local member=Connected cookie=4294967295 reply_cookie=0 signature=n/a error-name=n/a error-message=n/a stage 3: sd_bus_process returned 1 Got message type=signal sender=org.freedesktop.DBus destination=:1.733 path=/org/freedesktop/DBus interface=org.freedesktop.DBus member=NameAcquired cookie=4294967295 reply_cookie=0 signature=s error-name=n/a error-message=n/a stage 4: sd_bus_process returned 1 Got message type=error sender=org.freedesktop.DBus destination=:1.733 path=n/a interface=n/a member=n/a cookie=4294967295 reply_cookie=2 signature=s error-name=org.freedesktop.DBus.Error.AccessDenied error-message=Request to own name refused by policy Unable to request name, will retry after reloading DBus configuration: Request to own name refused by policy Sent message type=method_call sender=n/a destination=org.freedesktop.DBus path=/org/freedesktop/DBus interface=org.freedesktop.DBus member=ReloadConfig cookie=3 reply_cookie=0 signature=n/a error-name=n/a error-message=n/a request_name_destroy_callback n_ref=2 stage 5: sd_bus_process returned 1 Got message type=method_return sender=org.freedesktop.DBus destination=:1.733 path=n/a interface=n/a member=n/a cookie=4294967295 reply_cookie=3 signature= error-name=n/a error-message=n/a Sent message type=method_call sender=n/a destination=org.freedesktop.DBus path=/org/freedesktop/DBus interface=org.freedesktop.DBus member=RequestName cookie=4 reply_cookie=0 signature=su error-name=n/a error-message=n/a request_name_destroy_callback n_ref=1 stage 6: sd_bus_process returned 1 Got message type=error sender=org.freedesktop.DBus destination=:1.733 path=n/a interface=n/a member=n/a cookie=4294967295 reply_cookie=4 signature=s error-name=org.freedesktop.DBus.Error.AccessDenied error-message=Request to own name refused by policy Unable to request name, failing connection: Request to own name refused by policy Bus test-bus: changing state RUNNING → CLOSING stage 7: sd_bus_process returned 1 Bus test-bus: changing state CLOSING → CLOSED stage 8: sd_bus_process returned 1 stage 9: sd_bus_process returned -104 Processing failed: Connection reset by peer --- src/shared/bus-util.c | 62 ++++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 51 insertions(+), 11 deletions(-) (limited to 'src/shared/bus-util.c') diff --git a/src/shared/bus-util.c b/src/shared/bus-util.c index 018ffc387..020fa885a 100644 --- a/src/shared/bus-util.c +++ b/src/shared/bus-util.c @@ -1747,19 +1747,34 @@ int bus_open_system_watch_bind_with_description(sd_bus **ret, const char *descri } struct request_name_data { + unsigned n_ref; + const char *name; uint64_t flags; void *userdata; }; +static void request_name_destroy_callback(void *userdata) { + struct request_name_data *data = userdata; + + assert(data); + assert(data->n_ref > 0); + + log_info("%s n_ref=%u", __func__, data->n_ref); + + data->n_ref--; + if (data->n_ref == 0) + free(data); +} + static int reload_dbus_handler(sd_bus_message *m, void *userdata, sd_bus_error *ret_error) { - _cleanup_free_ struct request_name_data *data = userdata; + struct request_name_data *data = userdata; const sd_bus_error *e; int r; - assert(m); assert(data); assert(data->name); + assert(data->n_ref > 0); e = sd_bus_message_get_error(m); if (e) { @@ -1776,15 +1791,16 @@ static int reload_dbus_handler(sd_bus_message *m, void *userdata, sd_bus_error * } static int request_name_handler_may_reload_dbus(sd_bus_message *m, void *userdata, sd_bus_error *ret_error) { - _cleanup_free_ struct request_name_data *data = userdata; + struct request_name_data *data = userdata; uint32_t ret; int r; assert(m); - assert(userdata); + assert(data); if (sd_bus_message_is_method_error(m, NULL)) { const sd_bus_error *e = sd_bus_message_get_error(m); + _cleanup_(sd_bus_slot_unrefp) sd_bus_slot *slot = NULL; if (!sd_bus_error_has_name(e, SD_BUS_ERROR_ACCESS_DENIED)) { log_debug_errno(sd_bus_error_get_errno(e), @@ -1796,31 +1812,37 @@ static int request_name_handler_may_reload_dbus(sd_bus_message *m, void *userdat } log_debug_errno(sd_bus_error_get_errno(e), - "Unable to request name, retry after reloading DBus configuration: %s", + "Unable to request name, will retry after reloading DBus configuration: %s", e->message); /* If systemd-timesyncd.service enables DynamicUser= and dbus.service * started before the dynamic user is realized, then the DBus policy * about timesyncd has not been enabled yet. So, let's try to reload - * DBus configuration, and after that request name again. Note that it + * DBus configuration, and after that request the name again. Note that it * seems that no privileges are necessary to call the following method. */ r = sd_bus_call_method_async( sd_bus_message_get_bus(m), - NULL, + &slot, "org.freedesktop.DBus", "/org/freedesktop/DBus", "org.freedesktop.DBus", "ReloadConfig", reload_dbus_handler, - userdata, NULL); + data, NULL); if (r < 0) { log_error_errno(r, "Failed to reload DBus configuration: %m"); bus_enter_closing(sd_bus_message_get_bus(m)); return 1; } - data = NULL; /* Avoid free() */ + data->n_ref ++; + assert_se(sd_bus_slot_set_destroy_callback(slot, request_name_destroy_callback) >= 0); + + r = sd_bus_slot_set_floating(slot, true); + if (r < 0) + return r; + return 1; } @@ -1854,19 +1876,37 @@ static int request_name_handler_may_reload_dbus(sd_bus_message *m, void *userdat } int bus_request_name_async_may_reload_dbus(sd_bus *bus, sd_bus_slot **ret_slot, const char *name, uint64_t flags, void *userdata) { - struct request_name_data *data; + _cleanup_free_ struct request_name_data *data = NULL; + _cleanup_(sd_bus_slot_unrefp) sd_bus_slot *slot = NULL; + int r; data = new(struct request_name_data, 1); if (!data) return -ENOMEM; *data = (struct request_name_data) { + .n_ref = 1, .name = name, .flags = flags, .userdata = userdata, }; - return sd_bus_request_name_async(bus, ret_slot, name, flags, request_name_handler_may_reload_dbus, data); + r = sd_bus_request_name_async(bus, &slot, name, flags, request_name_handler_may_reload_dbus, data); + if (r < 0) + return r; + + assert_se(sd_bus_slot_set_destroy_callback(slot, request_name_destroy_callback) >= 0); + TAKE_PTR(data); + + if (ret_slot) + *ret_slot = TAKE_PTR(slot); + else { + r = sd_bus_slot_set_floating(slot, true); + if (r < 0) + return r; + } + + return 0; } int bus_reply_pair_array(sd_bus_message *m, char **l) { -- cgit v1.2.3