1
1
mirror of https://github.com/systemd/systemd-stable.git synced 2025-01-22 22:03:43 +03:00

shared/bus-util: use the new cleanup functionality to avoid a memleak

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
This commit is contained in:
Zbigniew Jędrzejewski-Szmek 2018-06-06 11:12:25 +02:00
parent fa17b4e8d9
commit 200bc75db9

View File

@ -1729,19 +1729,34 @@ int bus_open_system_watch_bind_with_description(sd_bus **ret, const char *descri
} }
struct request_name_data { struct request_name_data {
unsigned n_ref;
const char *name; const char *name;
uint64_t flags; uint64_t flags;
void *userdata; 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) { 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; const sd_bus_error *e;
int r; int r;
assert(m);
assert(data); assert(data);
assert(data->name); assert(data->name);
assert(data->n_ref > 0);
e = sd_bus_message_get_error(m); e = sd_bus_message_get_error(m);
if (e) { if (e) {
@ -1758,15 +1773,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) { 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; uint32_t ret;
int r; int r;
assert(m); assert(m);
assert(userdata); assert(data);
if (sd_bus_message_is_method_error(m, NULL)) { if (sd_bus_message_is_method_error(m, NULL)) {
const sd_bus_error *e = sd_bus_message_get_error(m); 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)) { if (!sd_bus_error_has_name(e, SD_BUS_ERROR_ACCESS_DENIED)) {
log_debug_errno(sd_bus_error_get_errno(e), log_debug_errno(sd_bus_error_get_errno(e),
@ -1778,31 +1794,37 @@ static int request_name_handler_may_reload_dbus(sd_bus_message *m, void *userdat
} }
log_debug_errno(sd_bus_error_get_errno(e), 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); e->message);
/* If systemd-timesyncd.service enables DynamicUser= and dbus.service /* If systemd-timesyncd.service enables DynamicUser= and dbus.service
* started before the dynamic user is realized, then the DBus policy * started before the dynamic user is realized, then the DBus policy
* about timesyncd has not been enabled yet. So, let's try to reload * 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. */ * seems that no privileges are necessary to call the following method. */
r = sd_bus_call_method_async( r = sd_bus_call_method_async(
sd_bus_message_get_bus(m), sd_bus_message_get_bus(m),
NULL, &slot,
"org.freedesktop.DBus", "org.freedesktop.DBus",
"/org/freedesktop/DBus", "/org/freedesktop/DBus",
"org.freedesktop.DBus", "org.freedesktop.DBus",
"ReloadConfig", "ReloadConfig",
reload_dbus_handler, reload_dbus_handler,
userdata, NULL); data, NULL);
if (r < 0) { if (r < 0) {
log_error_errno(r, "Failed to reload DBus configuration: %m"); log_error_errno(r, "Failed to reload DBus configuration: %m");
bus_enter_closing(sd_bus_message_get_bus(m)); bus_enter_closing(sd_bus_message_get_bus(m));
return 1; 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; return 1;
} }
@ -1836,19 +1858,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) { 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); data = new(struct request_name_data, 1);
if (!data) if (!data)
return -ENOMEM; return -ENOMEM;
*data = (struct request_name_data) { *data = (struct request_name_data) {
.n_ref = 1,
.name = name, .name = name,
.flags = flags, .flags = flags,
.userdata = userdata, .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) { int bus_reply_pair_array(sd_bus_message *m, char **l) {