--- ell/dbus.c | 96 +++++++++++++++++++++++++++++++++++++++++--------------------- 1 file changed, 63 insertions(+), 33 deletions(-) diff --git a/ell/dbus.c b/ell/dbus.c index 7cb4740..69222e9 100644 --- a/ell/dbus.c +++ b/ell/dbus.c @@ -96,6 +96,7 @@ struct l_dbus { struct _dbus_object_tree *tree; struct _dbus_name_cache *name_cache; struct _dbus_filter *filter; + bool name_notify_enabled; const struct l_dbus_ops *driver; }; @@ -545,11 +546,6 @@ static void dbus_init(struct l_dbus *dbus, int fd) dbus->signal_list = l_hashmap_new(); dbus->tree = _dbus_object_tree_new(); - - dbus->name_cache = _dbus_name_cache_new(dbus, &dbus->driver->name_ops); - - dbus->filter = _dbus_filter_new(dbus, &dbus->driver->filter_ops, - dbus->name_cache); } static void classic_free(struct l_dbus *dbus) @@ -742,6 +738,18 @@ static bool classic_remove_match(struct l_dbus *dbus, unsigned int id) return true; } +static void name_owner_changed_cb(struct l_dbus_message *message, + void *user_data) +{ + struct l_dbus *dbus = user_data; + char *name, *old, *new; + + if (!l_dbus_message_get_arguments(message, "sss", &name, &old, &new)) + return; + + _dbus_name_cache_notify(dbus->name_cache, name, new); +} + struct get_name_owner_request { struct l_dbus_message *message; struct l_dbus *dbus; @@ -789,6 +797,26 @@ static bool classic_get_name_owner(struct l_dbus *bus, const char *name) send_message(bus, false, req->message, get_name_owner_reply_cb, req, l_free); + if (!bus->name_notify_enabled) { + static struct _dbus_filter_condition rule[] = { + { L_DBUS_MATCH_TYPE, "signal" }, + { L_DBUS_MATCH_SENDER, DBUS_SERVICE_DBUS }, + { L_DBUS_MATCH_PATH, DBUS_PATH_DBUS }, + { L_DBUS_MATCH_INTERFACE, L_DBUS_INTERFACE_DBUS }, + { L_DBUS_MATCH_MEMBER, "NameOwnerChanged" }, + }; + + if (!bus->filter) + bus->filter = _dbus_filter_new(bus, + &bus->driver->filter_ops, + bus->name_cache); + + _dbus_filter_add_rule(bus->filter, rule, L_ARRAY_SIZE(rule), + name_owner_changed_cb, bus); + + bus->name_notify_enabled = true; + } + return true; } @@ -887,18 +915,6 @@ static const struct l_dbus_ops classic_ops = { .name_acquire = classic_name_acquire, }; -static void name_owner_changed_cb(struct l_dbus_message *message, - void *user_data) -{ - struct l_dbus *dbus = user_data; - char *name, *old, *new; - - if (!l_dbus_message_get_arguments(message, "sss", &name, &old, &new)) - return; - - _dbus_name_cache_notify(dbus->name_cache, name, new); -} - static struct l_dbus *setup_dbus1(int fd, const char *guid) { static const unsigned char creds = 0x00; @@ -908,13 +924,6 @@ static struct l_dbus *setup_dbus1(int fd, const char *guid) ssize_t written; unsigned int i; long flags; - static struct _dbus_filter_condition rule[] = { - { L_DBUS_MATCH_TYPE, "signal" }, - { L_DBUS_MATCH_SENDER, DBUS_SERVICE_DBUS }, - { L_DBUS_MATCH_PATH, DBUS_PATH_DBUS }, - { L_DBUS_MATCH_INTERFACE, L_DBUS_INTERFACE_DBUS }, - { L_DBUS_MATCH_MEMBER, "NameOwnerChanged" }, - }; if (snprintf(uid, sizeof(uid), "%d", geteuid()) < 1) { close(fd); @@ -963,9 +972,6 @@ static struct l_dbus *setup_dbus1(int fd, const char *guid) l_io_set_read_handler(dbus->io, auth_read_handler, dbus, NULL); l_io_set_write_handler(dbus->io, auth_write_handler, dbus, NULL); - _dbus_filter_add_rule(dbus->filter, rule, L_ARRAY_SIZE(rule), - name_owner_changed_cb, dbus); - return dbus; } @@ -1167,6 +1173,7 @@ static bool kdbus_get_name_owner(struct l_dbus *dbus, const char *name) int fd = l_io_get_fd(dbus->io); uint64_t owner_id; char owner[32]; + int r; owner_id = _dbus_kernel_get_name_owner(fd, kdbus->kdbus_pool, name); @@ -1177,6 +1184,15 @@ static bool kdbus_get_name_owner(struct l_dbus *dbus, const char *name) _dbus_name_cache_notify(dbus->name_cache, name, owner); + if (!dbus->name_notify_enabled) { + r = _dbus_kernel_enable_name_owner_notify(fd); + if (r < 0) + l_util_debug(dbus->debug_handler, + dbus->debug_data, strerror(-r)); + + dbus->name_notify_enabled = true; + } + return true; } @@ -1225,7 +1241,6 @@ static struct l_dbus *setup_kdbus(int fd) { struct l_dbus *dbus; struct l_dbus_kdbus *kdbus; - int r; kdbus = l_new(struct l_dbus_kdbus, 1); dbus = &kdbus->super; @@ -1246,11 +1261,6 @@ static struct l_dbus *setup_kdbus(int fd) l_idle_oneshot(kdbus_ready, dbus, NULL); - r = _dbus_kernel_enable_name_owner_notify(fd); - if (r < 0) - l_util_debug(dbus->debug_handler, - dbus->debug_data, strerror(-r)); - return dbus; } @@ -1760,6 +1770,10 @@ LIB_EXPORT unsigned int l_dbus_add_service_watch(struct l_dbus *dbus, if (!name) return 0; + if (!dbus->name_cache) + dbus->name_cache = _dbus_name_cache_new(dbus, + &dbus->driver->name_ops); + return _dbus_name_cache_add_watch(dbus->name_cache, name, connect_func, disconnect_func, user_data, destroy); @@ -1767,6 +1781,9 @@ LIB_EXPORT unsigned int l_dbus_add_service_watch(struct l_dbus *dbus, LIB_EXPORT bool l_dbus_remove_watch(struct l_dbus *dbus, unsigned int id) { + if (!dbus->name_cache) + return false; + return _dbus_name_cache_remove_watch(dbus->name_cache, id); } @@ -1876,6 +1893,16 @@ LIB_EXPORT unsigned int l_dbus_add_signal_watch(struct l_dbus *dbus, va_end(args); + if (!dbus->filter) { + if (!dbus->name_cache) + dbus->name_cache = _dbus_name_cache_new(dbus, + &dbus->driver->name_ops); + + dbus->filter = _dbus_filter_new(dbus, + &dbus->driver->filter_ops, + dbus->name_cache); + } + id = _dbus_filter_add_rule(dbus->filter, rule, rule_len, signal_func, user_data); @@ -1886,6 +1913,9 @@ LIB_EXPORT unsigned int l_dbus_add_signal_watch(struct l_dbus *dbus, LIB_EXPORT bool l_dbus_remove_signal_watch(struct l_dbus *dbus, unsigned int id) { + if (!dbus->filter) + return false; + return _dbus_filter_remove_rule(dbus->filter, id); } -- 2.7.4

--- ell/dbus-message.c | 17 +++++++++++++++++ ell/dbus-private.h | 2 ++ 2 files changed, 19 insertions(+) diff --git a/ell/dbus-message.c b/ell/dbus-message.c index 89aaacc..072a902 100644 --- a/ell/dbus-message.c +++ b/ell/dbus-message.c @@ -694,6 +694,23 @@ static bool valid_header(const struct dbus_header *hdr) return true; } +unsigned int _dbus_message_unix_fds_from_header(const void *data, size_t size) +{ + struct l_dbus_message message; + uint32_t unix_fds; + + message.header = (uint8_t *) data; + message.header_size = size; + message.body_size = 0; + message.sealed = true; + + if (!get_header_field(&message, DBUS_MESSAGE_FIELD_UNIX_FDS, + 'u', &unix_fds)) + return 0; + + return unix_fds; +} + struct l_dbus_message *dbus_message_from_blob(const void *data, size_t size, int fds[], uint32_t num_fds) { diff --git a/ell/dbus-private.h b/ell/dbus-private.h index 919639b..a4118c6 100644 --- a/ell/dbus-private.h +++ b/ell/dbus-private.h @@ -159,6 +159,8 @@ bool dbus_message_compare(struct l_dbus_message *message, bool _dbus_message_builder_mark(struct l_dbus_message_builder *builder); bool _dbus_message_builder_rewind(struct l_dbus_message_builder *builder); +unsigned int _dbus_message_unix_fds_from_header(const void *data, size_t size); + const char *_dbus_signature_end(const char *signature); bool _dbus_valid_object_path(const char *path); -- 2.7.4

We need to handle partial reads and writes and FD passing with multiple messages in the socket's queue so we may need multiple calls to send or recv. With a blocking socket we can call them in the loop and simplify buffering of message data. --- ell/dbus.c | 220 +++++++++++++++++++++++++++++++++++++++++++------------------ 1 file changed, 157 insertions(+), 63 deletions(-) diff --git a/ell/dbus.c b/ell/dbus.c index 69222e9..f6ae531 100644 --- a/ell/dbus.c +++ b/ell/dbus.c @@ -114,6 +114,8 @@ struct l_dbus_classic { void *auth_command; enum auth_state auth_state; struct l_hashmap *match_strings; + int *fd_buf; + unsigned int num_fds; }; struct message_callback { @@ -397,6 +399,12 @@ static bool auth_write_handler(struct l_io *io, void *user_data) l_util_hexdump(false, classic->auth_command, written, dbus->debug_handler, dbus->debug_data); + if (written < len) { + memmove(classic->auth_command, classic->auth_command + written, + len + 1 - written); + return true; + } + l_free(classic->auth_command); classic->auth_command = NULL; @@ -436,9 +444,13 @@ static bool auth_read_handler(struct l_io *io, void *user_data) offset = 0; while (1) { - len = recv(fd, ptr + offset, sizeof(buffer) - offset, 0); - if (len < 1) + len = recv(fd, ptr + offset, sizeof(buffer) - offset, + MSG_DONTWAIT); + if (len < 1) { + if (len == -1 && errno == EINTR) + continue; break; + } offset += len; } @@ -552,6 +564,11 @@ static void classic_free(struct l_dbus *dbus) { struct l_dbus_classic *classic = container_of(dbus, struct l_dbus_classic, super); + unsigned int i; + + for (i = 0; i < classic->num_fds; i++) + close(classic->fd_buf[i]); + l_free(classic->fd_buf); l_free(classic->auth_command); l_hashmap_destroy(classic->match_strings, l_free); @@ -563,48 +580,79 @@ static bool classic_send_message(struct l_dbus *dbus, { int fd = l_io_get_fd(dbus->io); struct msghdr msg; - struct iovec iov[2]; - ssize_t len; + struct iovec iov[2], *iovpos; + ssize_t r; int *fds; uint32_t num_fds = 0; struct cmsghdr *cmsg; + int iovlen; iov[0].iov_base = _dbus_message_get_header(message, &iov[0].iov_len); iov[1].iov_base = _dbus_message_get_body(message, &iov[1].iov_len); - memset(&msg, 0, sizeof(msg)); - msg.msg_iov = iov; - msg.msg_iovlen = 2; - if (dbus->support_unix_fd) fds = _dbus_message_get_fds(message, &num_fds); - if (num_fds) { - msg.msg_control = alloca(CMSG_SPACE(num_fds * sizeof(int))); - msg.msg_controllen = CMSG_LEN(num_fds * sizeof(int)); + iovpos = iov; + iovlen = 2; - cmsg = CMSG_FIRSTHDR(&msg); - cmsg->cmsg_len = msg.msg_controllen; - cmsg->cmsg_level = SOL_SOCKET; - cmsg->cmsg_type = SCM_RIGHTS; - memcpy(CMSG_DATA(cmsg), fds, num_fds * sizeof(int)); - } + while (1) { + memset(&msg, 0, sizeof(msg)); + msg.msg_iov = iovpos; + msg.msg_iovlen = iovlen; + + if (num_fds) { + msg.msg_control = + alloca(CMSG_SPACE(num_fds * sizeof(int))); + msg.msg_controllen = CMSG_LEN(num_fds * sizeof(int)); + + cmsg = CMSG_FIRSTHDR(&msg); + cmsg->cmsg_len = msg.msg_controllen; + cmsg->cmsg_level = SOL_SOCKET; + cmsg->cmsg_type = SCM_RIGHTS; + memcpy(CMSG_DATA(cmsg), fds, num_fds * sizeof(int)); + } - len = sendmsg(fd, &msg, 0); - if (len < 0) - return false; + r = sendmsg(fd, &msg, 0); + if (r <= 0) { + if (r < 0 && errno != EINTR) + return false; + + continue; + } + + while ((size_t) r >= iovpos->iov_len) { + r -= iovpos->iov_len; + iovpos++; + iovlen--; + + if (!iovlen) + break; + } + + if (!iovlen) + break; + + iovpos->iov_base += r; + iovpos->iov_len -= r; + + /* The FDs have been transmitted, don't retransmit */ + num_fds = 0; + } return true; } static struct l_dbus_message *classic_recv_message(struct l_dbus *dbus) { + struct l_dbus_classic *classic = + container_of(dbus, struct l_dbus_classic, super); int fd = l_io_get_fd(dbus->io); struct dbus_header hdr; struct msghdr msg; - struct iovec iov[2]; + struct iovec iov[2], *iovpos; struct cmsghdr *cmsg; - ssize_t len; + ssize_t len, r; void *header, *body; size_t header_size, body_size; union { @@ -613,6 +661,9 @@ static struct l_dbus_message *classic_recv_message(struct l_dbus *dbus) } fd_buf; int *fds = NULL; uint32_t num_fds = 0; + int iovlen; + struct l_dbus_message *message; + unsigned int i; len = recv(fd, &hdr, DBUS_HEADER_SIZE, MSG_PEEK); if (len != DBUS_HEADER_SIZE) @@ -629,38 +680,68 @@ static struct l_dbus_message *classic_recv_message(struct l_dbus *dbus) iov[1].iov_base = body; iov[1].iov_len = body_size; - memset(&msg, 0, sizeof(msg)); - msg.msg_iov = iov; - msg.msg_iovlen = 2; - msg.msg_control = &fd_buf; - msg.msg_controllen = sizeof(fd_buf); + iovpos = iov; + iovlen = 2; - len = recvmsg(fd, &msg, MSG_CMSG_CLOEXEC); - if (len < 0) - goto cmsg_fail; + while (1) { + memset(&msg, 0, sizeof(msg)); + msg.msg_iov = iovpos; + msg.msg_iovlen = iovlen; + msg.msg_control = &fd_buf; + msg.msg_controllen = sizeof(fd_buf); - for (cmsg = CMSG_FIRSTHDR(&msg); cmsg; - cmsg = CMSG_NXTHDR(&msg, cmsg)) { - unsigned int i; + r = recvmsg(fd, &msg, MSG_CMSG_CLOEXEC | MSG_WAITALL); + if (r < 0) { + if (errno != EINTR) + goto cmsg_fail; - if (cmsg->cmsg_level != SOL_SOCKET || - cmsg->cmsg_type != SCM_RIGHTS) continue; + } + + for (cmsg = CMSG_FIRSTHDR(&msg); cmsg; + cmsg = CMSG_NXTHDR(&msg, cmsg)) { + if (cmsg->cmsg_level != SOL_SOCKET || + cmsg->cmsg_type != SCM_RIGHTS) + continue; - num_fds = (cmsg->cmsg_len - CMSG_LEN(0)) / sizeof(int); - fds = (void *) CMSG_DATA(cmsg); + num_fds = (cmsg->cmsg_len - CMSG_LEN(0)) / sizeof(int); + fds = (void *) CMSG_DATA(cmsg); - /* Set FD_CLOEXEC on all file descriptors */ - for (i = 0; i < num_fds; i++) { - long flags; + /* Set FD_CLOEXEC on all file descriptors */ + for (i = 0; i < num_fds; i++) { + long flags; - flags = fcntl(fds[i], F_GETFD, NULL); - if (flags < 0) - continue; + flags = fcntl(fds[i], F_GETFD, NULL); + if (flags < 0) + continue; + + if (!(flags & FD_CLOEXEC)) + fcntl(fds[i], F_SETFD, + flags | FD_CLOEXEC); + } + + classic->fd_buf = l_realloc(classic->fd_buf, + (classic->num_fds + num_fds) * + sizeof(int)); + memcpy(classic->fd_buf + classic->num_fds, fds, + num_fds * sizeof(int)); + classic->num_fds += num_fds; + } - if (!(flags & FD_CLOEXEC)) - fcntl(fds[i], F_SETFD, flags | FD_CLOEXEC); - } + while ((size_t) r >= iovpos->iov_len) { + r -= iovpos->iov_len; + iovpos++; + iovlen--; + + if (!iovlen) + break; + } + + if (!iovlen) + break; + + iovpos->iov_base += r; + iovpos->iov_len -= r; } if (hdr.endian != DBUS_NATIVE_ENDIAN) { @@ -675,11 +756,39 @@ static struct l_dbus_message *classic_recv_message(struct l_dbus *dbus) goto bad_msg; } - return dbus_message_build(header, header_size, body, body_size, - fds, num_fds); + num_fds = _dbus_message_unix_fds_from_header(header, header_size); + if (num_fds > classic->num_fds) + goto bad_msg; + + message = dbus_message_build(header, header_size, body, body_size, + classic->fd_buf, num_fds); + + if (message && num_fds) { + if (classic->num_fds > num_fds) { + memmove(classic->fd_buf, classic->fd_buf + num_fds, + (classic->num_fds - num_fds) * sizeof(int)); + classic->num_fds -= num_fds; + } else { + l_free(classic->fd_buf); + + classic->fd_buf = NULL; + classic->num_fds = 0; + } + } + + if (message) + return message; bad_msg: cmsg_fail: + for (i = 0; i < classic->num_fds; i++) + close(classic->fd_buf[i]); + + l_free(classic->fd_buf); + + classic->fd_buf = NULL; + classic->num_fds = 0; + l_free(header); l_free(body); @@ -923,7 +1032,6 @@ static struct l_dbus *setup_dbus1(int fd, const char *guid) struct l_dbus_classic *classic; ssize_t written; unsigned int i; - long flags; if (snprintf(uid, sizeof(uid), "%d", geteuid()) < 1) { close(fd); @@ -940,20 +1048,6 @@ static struct l_dbus *setup_dbus1(int fd, const char *guid) return NULL; } - flags = fcntl(fd, F_GETFL, NULL); - if (flags < 0) { - close(fd); - return NULL; - } - - /* Input handling requires non-blocking socket */ - if (!(flags & O_NONBLOCK)) { - if (fcntl(fd, F_SETFL, flags | O_NONBLOCK) < 0) { - close(fd); - return NULL; - } - } - classic = l_new(struct l_dbus_classic, 1); dbus = &classic->super; dbus->driver = &classic_ops; -- 2.7.4