From eb01ba5de14859d7a94835ab9299de40132d549a Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 16 May 2013 21:14:56 +0200 Subject: [PATCH] bus: synthesize timeout message errors instead of returning error codes --- TODO | 16 ++++++ src/libsystemd-bus/bus-error.c | 80 ++++++++++++++++++++++++++--- src/libsystemd-bus/bus-match.c | 15 +++--- src/libsystemd-bus/bus-match.h | 2 +- src/libsystemd-bus/bus-message.c | 46 +++++++++++++++++ src/libsystemd-bus/bus-message.h | 4 ++ src/libsystemd-bus/sd-bus.c | 31 +++++++---- src/libsystemd-bus/test-bus-chat.c | 10 ++-- src/libsystemd-bus/test-bus-match.c | 6 +-- src/systemd/sd-bus.h | 9 +--- 10 files changed, 177 insertions(+), 42 deletions(-) diff --git a/TODO b/TODO index d862dfb50..14ed4b43c 100644 --- a/TODO +++ b/TODO @@ -29,6 +29,22 @@ Fedora 19: Features: +* libsystemd-bus: + - default policy (allow uid == 0 and our own uid) + - enforce alignment of pointers passed in + - negotiation for attach attributes + - verify that the PID doesn't change for existing busses + - when kdbus doesn't take our message without memfds, try again with memfds + - kdbus: generate correct bloom filter for matches + - implement translator service + - port systemd to new library + - implement busname unit type in systemd + - move to gvariant + - minimal locking around the memfd cache + - keep the connection fds around as long as the bus is open + - make ref counting atomic + - merge busctl into systemctl or so? + * in the final killing spree, detect processes from the root directory, and complain loudly if they have argv[0][0] == '@' set. https://bugzilla.redhat.com/show_bug.cgi?id=961044 diff --git a/src/libsystemd-bus/bus-error.c b/src/libsystemd-bus/bus-error.c index 5faa17384..4696a88f7 100644 --- a/src/libsystemd-bus/bus-error.c +++ b/src/libsystemd-bus/bus-error.c @@ -142,6 +142,9 @@ int bus_error_to_errno(const sd_bus_error* e) { /* Better replce this with a gperf table */ + if (!e) + return -EIO; + if (!e->name) return -EIO; @@ -152,6 +155,30 @@ int bus_error_to_errno(const sd_bus_error* e) { streq(e->name, "org.freedesktop.DBus.Error.AccessDenied")) return -EPERM; + if (streq(e->name, "org.freedesktop.DBus.Error.InvalidArgs")) + return -EINVAL; + + if (streq(e->name, "org.freedesktop.DBus.Error.UnixProcessIdUnknown")) + return -ESRCH; + + if (streq(e->name, "org.freedesktop.DBus.Error.FileNotFound")) + return -ENOENT; + + if (streq(e->name, "org.freedesktop.DBus.Error.FileExists")) + return -EEXIST; + + if (streq(e->name, "org.freedesktop.DBus.Error.Timeout")) + return -ETIMEDOUT; + + if (streq(e->name, "org.freedesktop.DBus.Error.IOError")) + return -EIO; + + if (streq(e->name, "org.freedesktop.DBus.Error.Disconnected")) + return -ECONNRESET; + + if (streq(e->name, "org.freedesktop.DBus.Error.NotSupported")) + return -ENOTSUP; + return -EIO; } @@ -159,13 +186,54 @@ int bus_error_from_errno(sd_bus_error *e, int error) { if (!e) return error; - if (error == -ENOMEM) - sd_bus_error_set_const(e, "org.freedesktop.DBus.Error.NoMemory", strerror(-error)); - else if (error == -EPERM || error == -EACCES) - sd_bus_error_set_const(e, "org.freedesktop.DBus.Error.AccessDenied", strerror(-error)); - else - sd_bus_error_set_const(e, "org.freedesktop.DBus.Error.Failed", "Operation failed"); + switch (error) { + + case -ENOMEM: + sd_bus_error_set_const(e, "org.freedesktop.DBus.Error.NoMemory", "Out of memory"); + break; + + case -EPERM: + case -EACCES: + sd_bus_error_set_const(e, "org.freedesktop.DBus.Error.AccessDenied", "Access denied"); + break; + + case -EINVAL: + sd_bus_error_set_const(e, "org.freedesktop.DBus.Error.InvalidArgs", "Invalid argument"); + break; + + case -ESRCH: + sd_bus_error_set_const(e, "org.freedesktop.DBus.Error.UnixProcessIdUnknown", "No such process"); + break; + + case -ENOENT: + sd_bus_error_set_const(e, "org.freedesktop.DBus.Error.FileNotFound", "File not found"); + break; + + case -EEXIST: + sd_bus_error_set_const(e, "org.freedesktop.DBus.Error.FileExists", "File exists"); + break; + + case -ETIMEDOUT: + case -ETIME: + sd_bus_error_set_const(e, "org.freedesktop.DBus.Error.Timeout", "Timed out"); + break; + + case -EIO: + sd_bus_error_set_const(e, "org.freedesktop.DBus.Error.IOError", "Input/output error"); + break; + + case -ENETRESET: + case -ECONNABORTED: + case -ECONNRESET: + sd_bus_error_set_const(e, "org.freedesktop.DBus.Error.Disconnected", "Disconnected"); + break; + + case -ENOTSUP: + sd_bus_error_set_const(e, "org.freedesktop.DBus.Error.NotSupported", "Not supported"); + break; + } + sd_bus_error_set_const(e, "org.freedesktop.DBus.Error.Failed", "Operation failed"); return error; } diff --git a/src/libsystemd-bus/bus-match.c b/src/libsystemd-bus/bus-match.c index 501a38df7..ed871d9d7 100644 --- a/src/libsystemd-bus/bus-match.c +++ b/src/libsystemd-bus/bus-match.c @@ -199,7 +199,6 @@ static bool value_node_same( int bus_match_run( sd_bus *bus, struct bus_match_node *node, - int ret, sd_bus_message *m) { @@ -230,7 +229,7 @@ int bus_match_run( * we won't call any. The children of the root node * are compares or leaves, they will automatically * call their siblings. */ - return bus_match_run(bus, node->child, ret, m); + return bus_match_run(bus, node->child, m); case BUS_MATCH_VALUE: @@ -240,7 +239,7 @@ int bus_match_run( * automatically call their siblings */ assert(node->child); - return bus_match_run(bus, node->child, ret, m); + return bus_match_run(bus, node->child, m); case BUS_MATCH_LEAF: @@ -257,11 +256,11 @@ int bus_match_run( /* Run the callback. And then invoke siblings. */ assert(node->leaf.callback); - r = node->leaf.callback(bus, ret, m, node->leaf.userdata); + r = node->leaf.callback(bus, m, node->leaf.userdata); if (r != 0) return r; - return bus_match_run(bus, node->next, ret, m); + return bus_match_run(bus, node->next, m); case BUS_MATCH_MESSAGE_TYPE: test_u8 = m->header->type; @@ -318,7 +317,7 @@ int bus_match_run( found = NULL; if (found) { - r = bus_match_run(bus, found, ret, m); + r = bus_match_run(bus, found, m); if (r != 0) return r; } @@ -331,7 +330,7 @@ int bus_match_run( if (!value_node_test(c, node->type, test_u8, test_str)) continue; - r = bus_match_run(bus, c, ret, m); + r = bus_match_run(bus, c, m); if (r != 0) return r; } @@ -341,7 +340,7 @@ int bus_match_run( return 0; /* And now, let's invoke our siblings */ - return bus_match_run(bus, node->next, ret, m); + return bus_match_run(bus, node->next, m); } static int bus_match_add_compare_value( diff --git a/src/libsystemd-bus/bus-match.h b/src/libsystemd-bus/bus-match.h index 075f1a9e3..4d46cf9af 100644 --- a/src/libsystemd-bus/bus-match.h +++ b/src/libsystemd-bus/bus-match.h @@ -69,7 +69,7 @@ struct bus_match_node { }; }; -int bus_match_run(sd_bus *bus, struct bus_match_node *root, int ret, sd_bus_message *m); +int bus_match_run(sd_bus *bus, struct bus_match_node *root, sd_bus_message *m); int bus_match_add(struct bus_match_node *root, const char *match, sd_bus_message_handler_t callback, void *userdata, struct bus_match_node **ret); int bus_match_remove(struct bus_match_node *root, const char *match, sd_bus_message_handler_t callback, void *userdata); diff --git a/src/libsystemd-bus/bus-message.c b/src/libsystemd-bus/bus-message.c index dbb333761..c72b52d54 100644 --- a/src/libsystemd-bus/bus-message.c +++ b/src/libsystemd-bus/bus-message.c @@ -623,6 +623,43 @@ fail: return r; } +int bus_message_new_synthetic_error( + sd_bus *bus, + uint64_t serial, + const sd_bus_error *e, + sd_bus_message **m) { + + sd_bus_message *t; + int r; + + assert(sd_bus_error_is_set(e)); + assert(m); + + t = message_new(bus, SD_BUS_MESSAGE_TYPE_METHOD_ERROR); + if (!t) + return -ENOMEM; + + t->header->flags |= SD_BUS_MESSAGE_NO_REPLY_EXPECTED; + t->reply_serial = serial; + + r = message_append_field_uint32(t, SD_BUS_MESSAGE_HEADER_REPLY_SERIAL, t->reply_serial); + if (r < 0) + goto fail; + + if (bus && bus->unique_name) { + r = message_append_field_string(t, SD_BUS_MESSAGE_HEADER_DESTINATION, SD_BUS_TYPE_STRING, bus->unique_name, &t->destination); + if (r < 0) + goto fail; + } + + *m = t; + return 0; + +fail: + message_free(t); + return r; +} + sd_bus_message* sd_bus_message_ref(sd_bus_message *m) { if (!m) return NULL; @@ -4240,3 +4277,12 @@ int bus_header_message_size(struct bus_header *h, size_t *sum) { *sum = sizeof(struct bus_header) + ALIGN8(fs) + bs; return 0; } + +int bus_message_to_errno(sd_bus_message *m) { + assert(m); + + if (m->header->type != SD_BUS_MESSAGE_TYPE_METHOD_ERROR) + return 0; + + return bus_error_to_errno(&m->error); +} diff --git a/src/libsystemd-bus/bus-message.h b/src/libsystemd-bus/bus-message.h index 44390c6b5..2fb11ea3b 100644 --- a/src/libsystemd-bus/bus-message.h +++ b/src/libsystemd-bus/bus-message.h @@ -235,3 +235,7 @@ struct bus_body_part *message_append_part(sd_bus_message *m); int bus_body_part_map(struct bus_body_part *part); void bus_body_part_unmap(struct bus_body_part *part); + +int bus_message_to_errno(sd_bus_message *m); + +int bus_message_new_synthetic_error(sd_bus *bus, uint64_t serial, const sd_bus_error *e, sd_bus_message **m); diff --git a/src/libsystemd-bus/sd-bus.c b/src/libsystemd-bus/sd-bus.c index 2537ba52d..7b937d999 100644 --- a/src/libsystemd-bus/sd-bus.c +++ b/src/libsystemd-bus/sd-bus.c @@ -229,18 +229,18 @@ int sd_bus_set_anonymous(sd_bus *bus, int b) { return 0; } -static int hello_callback(sd_bus *bus, int error, sd_bus_message *reply, void *userdata) { +static int hello_callback(sd_bus *bus, sd_bus_message *reply, void *userdata) { const char *s; int r; assert(bus); assert(bus->state == BUS_HELLO); - - if (error != 0) - return -error; - assert(reply); + r = bus_message_to_errno(reply); + if (r < 0) + return r; + r = sd_bus_message_read(reply, "s", &s); if (r < 0) return r; @@ -1523,6 +1523,7 @@ int sd_bus_get_timeout(sd_bus *bus, uint64_t *timeout_usec) { } static int process_timeout(sd_bus *bus) { + _cleanup_bus_message_unref_ sd_bus_message* m = NULL; struct reply_callback *c; usec_t n; int r; @@ -1537,10 +1538,18 @@ static int process_timeout(sd_bus *bus) { if (c->timeout > n) return 0; + r = bus_message_new_synthetic_error( + bus, + c->serial, + &SD_BUS_ERROR_MAKE("org.freedesktop.DBus.Error.Timeout", "Timed out"), + &m); + if (r < 0) + return r; + assert_se(prioq_pop(bus->reply_callbacks_prioq) == c); hashmap_remove(bus->reply_callbacks, &c->serial); - r = c->callback(bus, ETIMEDOUT, NULL, c->userdata); + r = c->callback(bus, m, c->userdata); free(c); return r < 0 ? r : 1; @@ -1590,7 +1599,7 @@ static int process_reply(sd_bus *bus, sd_bus_message *m) { if (r < 0) return r; - r = c->callback(bus, 0, m, c->userdata); + r = c->callback(bus, m, c->userdata); free(c); return r; @@ -1621,7 +1630,7 @@ static int process_filter(sd_bus *bus, sd_bus_message *m) { if (r < 0) return r; - r = l->callback(bus, 0, m, l->userdata); + r = l->callback(bus, m, l->userdata); if (r != 0) return r; @@ -1641,7 +1650,7 @@ static int process_match(sd_bus *bus, sd_bus_message *m) { do { bus->match_callbacks_modified = false; - r = bus_match_run(bus, &bus->match_callbacks, 0, m); + r = bus_match_run(bus, &bus->match_callbacks, m); if (r != 0) return r; @@ -1734,7 +1743,7 @@ static int process_object(sd_bus *bus, sd_bus_message *m) { if (r < 0) return r; - r = c->callback(bus, 0, m, c->userdata); + r = c->callback(bus, m, c->userdata); if (r != 0) return r; @@ -1764,7 +1773,7 @@ static int process_object(sd_bus *bus, sd_bus_message *m) { if (r < 0) return r; - r = c->callback(bus, 0, m, c->userdata); + r = c->callback(bus, m, c->userdata); if (r != 0) return r; diff --git a/src/libsystemd-bus/test-bus-chat.c b/src/libsystemd-bus/test-bus-chat.c index f457c8f88..f308eddbb 100644 --- a/src/libsystemd-bus/test-bus-chat.c +++ b/src/libsystemd-bus/test-bus-chat.c @@ -35,17 +35,17 @@ #include "bus-match.h" #include "bus-internal.h" -static int match_callback(sd_bus *bus, int error, sd_bus_message *m, void *userdata) { +static int match_callback(sd_bus *bus, sd_bus_message *m, void *userdata) { log_info("Match triggered! interface=%s member=%s", strna(sd_bus_message_get_interface(m)), strna(sd_bus_message_get_member(m))); return 0; } -static int object_callback(sd_bus *bus, int error, sd_bus_message *m, void *userdata) { +static int object_callback(sd_bus *bus, sd_bus_message *m, void *userdata) { int r; assert(bus); - if (error != 0) + if (sd_bus_message_is_method_error(m, NULL)) return 0; if (sd_bus_message_is_method_call(m, "org.object.test", "Foobar")) { @@ -356,10 +356,10 @@ finish: return INT_TO_PTR(r); } -static int quit_callback(sd_bus *b, int ret, sd_bus_message *m, void *userdata) { +static int quit_callback(sd_bus *b, sd_bus_message *m, void *userdata) { bool *x = userdata; - log_error("Quit callback: %s", strerror(ret)); + log_error("Quit callback: %s", strerror(bus_message_to_errno(m))); *x = 1; return 1; diff --git a/src/libsystemd-bus/test-bus-match.c b/src/libsystemd-bus/test-bus-match.c index 9cf994009..605d86a85 100644 --- a/src/libsystemd-bus/test-bus-match.c +++ b/src/libsystemd-bus/test-bus-match.c @@ -30,7 +30,7 @@ static bool mask[32]; -static int filter(sd_bus *b, int ret, sd_bus_message *m, void *userdata) { +static int filter(sd_bus *b, sd_bus_message *m, void *userdata) { log_info("Ran %i", PTR_TO_INT(userdata)); mask[PTR_TO_INT(userdata)] = true; return 0; @@ -85,7 +85,7 @@ int main(int argc, char *argv[]) { assert_se(bus_message_seal(m, 1) >= 0); zero(mask); - assert_se(bus_match_run(NULL, &root, 0, m) == 0); + assert_se(bus_match_run(NULL, &root, m) == 0); assert_se(mask_contains((unsigned[]) { 9, 8, 7, 5, 10, 12, 13, 14 }, 8)); assert_se(bus_match_remove(&root, "member='waldo',path='/foo/bar'", filter, INT_TO_PTR(8)) > 0); @@ -95,7 +95,7 @@ int main(int argc, char *argv[]) { bus_match_dump(&root, 0); zero(mask); - assert_se(bus_match_run(NULL, &root, 0, m) == 0); + assert_se(bus_match_run(NULL, &root, m) == 0); assert_se(mask_contains((unsigned[]) { 9, 5, 10, 12, 14, 7 }, 6)); for (i = 0; i < _BUS_MATCH_NODE_TYPE_MAX; i++) { diff --git a/src/systemd/sd-bus.h b/src/systemd/sd-bus.h index 4c3c26d61..85deacf63 100644 --- a/src/systemd/sd-bus.h +++ b/src/systemd/sd-bus.h @@ -41,13 +41,6 @@ extern "C" { # endif #endif -/* TODO: - * - merge busctl into systemctl or so? - * - default policy (allow uid == 0 and our own uid) - * - enforce alignment of pointers passed in - * - negotiation for attach attributes - */ - typedef struct sd_bus sd_bus; typedef struct sd_bus_message sd_bus_message; @@ -57,7 +50,7 @@ typedef struct { int need_free; } sd_bus_error; -typedef int (*sd_bus_message_handler_t)(sd_bus *bus, int ret, sd_bus_message *m, void *userdata); +typedef int (*sd_bus_message_handler_t)(sd_bus *bus, sd_bus_message *m, void *userdata); /* Connections */ -- 2.30.2