From 501ecd670c2aeb9e92a7a7fa45a0172e808180ec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 19 Mar 2020 21:28:25 +0100 Subject: [PATCH 1/3] sd-bus: make sure SD_BUS_DEFAULT* works everywhere I'm not sure why those functions were not touched in 45b1f410ba699990e4033805fe64cc369b835f10. Anyway, it seems easier to just support the magic parameters everywhere. --- src/libsystemd/sd-bus/bus-convenience.c | 6 ++++++ src/libsystemd/sd-bus/bus-message.c | 3 +++ src/libsystemd/sd-bus/sd-bus.c | 10 +++++----- 3 files changed, 14 insertions(+), 5 deletions(-) diff --git a/src/libsystemd/sd-bus/bus-convenience.c b/src/libsystemd/sd-bus/bus-convenience.c index 4ec061644d..a1d81a1315 100644 --- a/src/libsystemd/sd-bus/bus-convenience.c +++ b/src/libsystemd/sd-bus/bus-convenience.c @@ -96,6 +96,7 @@ _public_ int sd_bus_call_method( int r; bus_assert_return(bus, -EINVAL, error); + bus_assert_return(bus = bus_resolve(bus), -ENOPKG, error); bus_assert_return(!bus_pid_changed(bus), -ECHILD, error); if (!BUS_IS_OPEN(bus->state)) { @@ -283,6 +284,7 @@ _public_ int sd_bus_get_property( int r; bus_assert_return(bus, -EINVAL, error); + bus_assert_return(bus = bus_resolve(bus), -ENOPKG, error); bus_assert_return(isempty(interface) || interface_name_is_valid(interface), -EINVAL, error); bus_assert_return(member_name_is_valid(member), -EINVAL, error); bus_assert_return(reply, -EINVAL, error); @@ -324,6 +326,7 @@ _public_ int sd_bus_get_property_trivial( int r; bus_assert_return(bus, -EINVAL, error); + bus_assert_return(bus = bus_resolve(bus), -ENOPKG, error); bus_assert_return(isempty(interface) || interface_name_is_valid(interface), -EINVAL, error); bus_assert_return(member_name_is_valid(member), -EINVAL, error); bus_assert_return(bus_type_is_trivial(type), -EINVAL, error); @@ -368,6 +371,7 @@ _public_ int sd_bus_get_property_string( int r; bus_assert_return(bus, -EINVAL, error); + bus_assert_return(bus = bus_resolve(bus), -ENOPKG, error); bus_assert_return(isempty(interface) || interface_name_is_valid(interface), -EINVAL, error); bus_assert_return(member_name_is_valid(member), -EINVAL, error); bus_assert_return(ret, -EINVAL, error); @@ -416,6 +420,7 @@ _public_ int sd_bus_get_property_strv( int r; bus_assert_return(bus, -EINVAL, error); + bus_assert_return(bus = bus_resolve(bus), -ENOPKG, error); bus_assert_return(isempty(interface) || interface_name_is_valid(interface), -EINVAL, error); bus_assert_return(member_name_is_valid(member), -EINVAL, error); bus_assert_return(ret, -EINVAL, error); @@ -458,6 +463,7 @@ _public_ int sd_bus_set_property( int r; bus_assert_return(bus, -EINVAL, error); + bus_assert_return(bus = bus_resolve(bus), -ENOPKG, error); bus_assert_return(isempty(interface) || interface_name_is_valid(interface), -EINVAL, error); bus_assert_return(member_name_is_valid(member), -EINVAL, error); bus_assert_return(signature_is_single(type, false), -EINVAL, error); diff --git a/src/libsystemd/sd-bus/bus-message.c b/src/libsystemd/sd-bus/bus-message.c index 73127dfe02..b8f91f4434 100644 --- a/src/libsystemd/sd-bus/bus-message.c +++ b/src/libsystemd/sd-bus/bus-message.c @@ -588,6 +588,7 @@ _public_ int sd_bus_message_new( sd_bus_message *t; assert_return(bus, -ENOTCONN); + assert_return(bus = bus_resolve(bus), -ENOPKG); assert_return(bus->state != BUS_UNSET, -ENOTCONN); assert_return(m, -EINVAL); assert_return(type < _SD_BUS_MESSAGE_TYPE_MAX, -EINVAL); @@ -623,6 +624,7 @@ _public_ int sd_bus_message_new_signal( int r; assert_return(bus, -ENOTCONN); + assert_return(bus = bus_resolve(bus), -ENOPKG); assert_return(bus->state != BUS_UNSET, -ENOTCONN); assert_return(object_path_is_valid(path), -EINVAL); assert_return(interface_name_is_valid(interface), -EINVAL); @@ -663,6 +665,7 @@ _public_ int sd_bus_message_new_method_call( int r; assert_return(bus, -ENOTCONN); + assert_return(bus = bus_resolve(bus), -ENOPKG); assert_return(bus->state != BUS_UNSET, -ENOTCONN); assert_return(!destination || service_name_is_valid(destination), -EINVAL); assert_return(object_path_is_valid(path), -EINVAL); diff --git a/src/libsystemd/sd-bus/sd-bus.c b/src/libsystemd/sd-bus/sd-bus.c index e45334c83f..acfff6d28e 100644 --- a/src/libsystemd/sd-bus/sd-bus.c +++ b/src/libsystemd/sd-bus/sd-bus.c @@ -3674,31 +3674,31 @@ _public_ int sd_bus_detach_event(sd_bus *bus) { } _public_ sd_event* sd_bus_get_event(sd_bus *bus) { - assert_return(bus, NULL); + assert_return(bus = bus_resolve(bus), NULL); return bus->event; } _public_ sd_bus_message* sd_bus_get_current_message(sd_bus *bus) { - assert_return(bus, NULL); + assert_return(bus = bus_resolve(bus), NULL); return bus->current_message; } _public_ sd_bus_slot* sd_bus_get_current_slot(sd_bus *bus) { - assert_return(bus, NULL); + assert_return(bus = bus_resolve(bus), NULL); return bus->current_slot; } _public_ sd_bus_message_handler_t sd_bus_get_current_handler(sd_bus *bus) { - assert_return(bus, NULL); + assert_return(bus = bus_resolve(bus), NULL); return bus->current_handler; } _public_ void* sd_bus_get_current_userdata(sd_bus *bus) { - assert_return(bus, NULL); + assert_return(bus = bus_resolve(bus), NULL); return bus->current_userdata; } From 70bc558cc10685b1e67f493415de17ebfc5138c5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 19 Mar 2020 21:30:44 +0100 Subject: [PATCH 2/3] sd-bus: support SD_BUS_DEFAULT* and don't crash in functions where bus is optional In those functions where bus defaults to the m->bus, we should also resolve the magic parameters. And if neither called with bus=NULL and an unattached message, return properly instead of crashing in assert later. --- src/libsystemd/sd-bus/sd-bus.c | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/src/libsystemd/sd-bus/sd-bus.c b/src/libsystemd/sd-bus/sd-bus.c index acfff6d28e..58c4571819 100644 --- a/src/libsystemd/sd-bus/sd-bus.c +++ b/src/libsystemd/sd-bus/sd-bus.c @@ -1899,9 +1899,10 @@ _public_ int sd_bus_send(sd_bus *bus, sd_bus_message *_m, uint64_t *cookie) { assert_return(m, -EINVAL); - if (!bus) - bus = m->bus; - + if (bus) + assert_return(bus = bus_resolve(bus), -ENOPKG); + else + assert_return(bus = m->bus, -ENOTCONN); assert_return(!bus_pid_changed(bus), -ECHILD); if (!BUS_IS_OPEN(bus->state)) @@ -1983,9 +1984,10 @@ _public_ int sd_bus_send_to(sd_bus *bus, sd_bus_message *m, const char *destinat assert_return(m, -EINVAL); - if (!bus) - bus = m->bus; - + if (bus) + assert_return(bus = bus_resolve(bus), -ENOPKG); + else + assert_return(bus = m->bus, -ENOTCONN); assert_return(!bus_pid_changed(bus), -ECHILD); if (!BUS_IS_OPEN(bus->state)) @@ -2048,9 +2050,10 @@ _public_ int sd_bus_call_async( assert_return(m->header->type == SD_BUS_MESSAGE_METHOD_CALL, -EINVAL); assert_return(!m->sealed || (!!callback == !(m->header->flags & BUS_MESSAGE_NO_REPLY_EXPECTED)), -EINVAL); - if (!bus) - bus = m->bus; - + if (bus) + assert_return(bus = bus_resolve(bus), -ENOPKG); + else + assert_return(bus = m->bus, -ENOTCONN); assert_return(!bus_pid_changed(bus), -ECHILD); if (!BUS_IS_OPEN(bus->state)) @@ -2154,9 +2157,10 @@ _public_ int sd_bus_call( bus_assert_return(!(m->header->flags & BUS_MESSAGE_NO_REPLY_EXPECTED), -EINVAL, error); bus_assert_return(!bus_error_is_dirty(error), -EINVAL, error); - if (!bus) - bus = m->bus; - + if (bus) + assert_return(bus = bus_resolve(bus), -ENOPKG); + else + assert_return(bus = m->bus, -ENOTCONN); bus_assert_return(!bus_pid_changed(bus), -ECHILD, error); if (!BUS_IS_OPEN(bus->state)) { From f41df6954ca1913aba1b1596c8f9ab4f74fa61dc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 19 Mar 2020 21:31:04 +0100 Subject: [PATCH 3/3] sd-bus: whitespace adjustments --- src/libsystemd/sd-bus/bus-convenience.c | 5 ++++- src/libsystemd/sd-bus/bus-message.c | 4 +--- src/libsystemd/sd-bus/sd-bus.c | 3 --- 3 files changed, 5 insertions(+), 7 deletions(-) diff --git a/src/libsystemd/sd-bus/bus-convenience.c b/src/libsystemd/sd-bus/bus-convenience.c index a1d81a1315..89547a252d 100644 --- a/src/libsystemd/sd-bus/bus-convenience.c +++ b/src/libsystemd/sd-bus/bus-convenience.c @@ -296,7 +296,10 @@ _public_ int sd_bus_get_property( goto fail; } - r = sd_bus_call_method(bus, destination, path, "org.freedesktop.DBus.Properties", "Get", error, &rep, "ss", strempty(interface), member); + r = sd_bus_call_method(bus, destination, path, + "org.freedesktop.DBus.Properties", "Get", + error, &rep, + "ss", strempty(interface), member); if (r < 0) return r; diff --git a/src/libsystemd/sd-bus/bus-message.c b/src/libsystemd/sd-bus/bus-message.c index b8f91f4434..62c34d9312 100644 --- a/src/libsystemd/sd-bus/bus-message.c +++ b/src/libsystemd/sd-bus/bus-message.c @@ -585,15 +585,13 @@ _public_ int sd_bus_message_new( sd_bus_message **m, uint8_t type) { - sd_bus_message *t; - assert_return(bus, -ENOTCONN); assert_return(bus = bus_resolve(bus), -ENOPKG); assert_return(bus->state != BUS_UNSET, -ENOTCONN); assert_return(m, -EINVAL); assert_return(type < _SD_BUS_MESSAGE_TYPE_MAX, -EINVAL); - t = malloc0(ALIGN(sizeof(sd_bus_message)) + sizeof(struct bus_header)); + sd_bus_message *t = malloc0(ALIGN(sizeof(sd_bus_message)) + sizeof(struct bus_header)); if (!t) return -ENOMEM; diff --git a/src/libsystemd/sd-bus/sd-bus.c b/src/libsystemd/sd-bus/sd-bus.c index 58c4571819..5cbd01594c 100644 --- a/src/libsystemd/sd-bus/sd-bus.c +++ b/src/libsystemd/sd-bus/sd-bus.c @@ -504,7 +504,6 @@ static int synthesize_connected_signal(sd_bus *bus) { } void bus_set_state(sd_bus *bus, enum bus_state state) { - static const char * const table[_BUS_STATE_MAX] = { [BUS_UNSET] = "UNSET", [BUS_WATCH_BIND] = "WATCH_BIND", @@ -2287,7 +2286,6 @@ fail: } _public_ int sd_bus_get_fd(sd_bus *bus) { - assert_return(bus, -EINVAL); assert_return(bus = bus_resolve(bus), -ENOPKG); assert_return(bus->input_fd == bus->output_fd, -EPERM); @@ -4019,7 +4017,6 @@ _public_ int sd_bus_get_scope(sd_bus *bus, const char **scope) { } _public_ int sd_bus_get_address(sd_bus *bus, const char **address) { - assert_return(bus, -EINVAL); assert_return(bus = bus_resolve(bus), -ENOPKG); assert_return(address, -EINVAL);