From d8fdc62037b5b0a9fd603ad5efd6b49f956f86b5 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 4 May 2016 20:43:23 +0200 Subject: core: use an AF_UNIX/SOCK_DGRAM socket for cgroup agent notification dbus-daemon currently uses a backlog of 30 on its D-bus system bus socket. On overloaded systems this means that only 30 connections may be queued without dbus-daemon processing them before further connection attempts fail. Our cgroups-agent binary so far used D-Bus for its messaging, and hitting this limit hence may result in us losing cgroup empty messages. This patch adds a seperate cgroup agent socket of type AF_UNIX/SOCK_DGRAM. Since sockets of these types need no connection set up, no listen() backlog applies. Our cgroup-agent binary will hence simply block as long as it can't enqueue its datagram message, so that we won't lose cgroup empty messages as likely anymore. This also rearranges the ordering of the processing of SIGCHLD signals, service notification messages (sd_notify()...) and the two types of cgroup notifications (inotify for the unified hierarchy support, and agent for the classic hierarchy support). We now always process events for these in the following order: 1. service notification messages (SD_EVENT_PRIORITY_NORMAL-7) 2. SIGCHLD signals (SD_EVENT_PRIORITY_NORMAL-6) 3. cgroup inotify and cgroup agent (SD_EVENT_PRIORITY_NORMAL-5) This is because when receiving SIGCHLD we invalidate PID information, which we need to process the service notification messages which are bound to PIDs. Hence the order between the first two items. And we want to process SIGCHLD metadata to detect whether a service is gone, before using cgroup notifications, to decide when a service is gone, since the former carries more useful metadata. Related to this: https://bugs.freedesktop.org/show_bug.cgi?id=95264 https://github.com/systemd/systemd/issues/1961 --- src/core/cgroup.c | 6 ++- src/core/dbus.c | 73 ++++++++++--------------- src/core/dbus.h | 2 + src/core/manager.c | 155 +++++++++++++++++++++++++++++++++++++++++++++++++---- src/core/manager.h | 3 ++ 5 files changed, 184 insertions(+), 55 deletions(-) (limited to 'src/core') diff --git a/src/core/cgroup.c b/src/core/cgroup.c index 25cc6962f9..1a94b188cb 100644 --- a/src/core/cgroup.c +++ b/src/core/cgroup.c @@ -1312,7 +1312,9 @@ int manager_setup_cgroup(Manager *m) { if (r < 0) return log_error_errno(r, "Failed to watch control group inotify object: %m"); - r = sd_event_source_set_priority(m->cgroup_inotify_event_source, SD_EVENT_PRIORITY_IDLE - 5); + /* Process cgroup empty notifications early, but after service notifications and SIGCHLD. Also + * see handling of cgroup agent notifications, for the classic cgroup hierarchy support. */ + r = sd_event_source_set_priority(m->cgroup_inotify_event_source, SD_EVENT_PRIORITY_NORMAL-5); if (r < 0) return log_error_errno(r, "Failed to set priority of inotify event source: %m"); @@ -1458,6 +1460,8 @@ int manager_notify_cgroup_empty(Manager *m, const char *cgroup) { assert(m); assert(cgroup); + log_debug("Got cgroup empty notification for: %s", cgroup); + u = manager_get_unit_by_cgroup(m, cgroup); if (!u) return 0; diff --git a/src/core/dbus.c b/src/core/dbus.c index 263955d874..c8375a0475 100644 --- a/src/core/dbus.c +++ b/src/core/dbus.c @@ -71,28 +71,42 @@ int bus_send_queued_message(Manager *m) { return 0; } +int bus_forward_agent_released(Manager *m, const char *path) { + int r; + + assert(m); + assert(path); + + if (!MANAGER_IS_SYSTEM(m)) + return 0; + + if (!m->system_bus) + return 0; + + /* If we are running a system instance we forward the agent message on the system bus, so that the user + * instances get notified about this, too */ + + r = sd_bus_emit_signal(m->system_bus, + "/org/freedesktop/systemd1/agent", + "org.freedesktop.systemd1.Agent", + "Released", + "s", path); + if (r < 0) + return log_warning_errno(r, "Failed to propagate agent release message: %m"); + + return 1; +} + static int signal_agent_released(sd_bus_message *message, void *userdata, sd_bus_error *error) { _cleanup_(sd_bus_creds_unrefp) sd_bus_creds *creds = NULL; - const char *cgroup, *me; Manager *m = userdata; + const char *cgroup; uid_t sender_uid; - sd_bus *bus; int r; assert(message); assert(m); - /* ignore recursive events sent by us on the system/user bus */ - bus = sd_bus_message_get_bus(message); - if (!sd_bus_is_server(bus)) { - r = sd_bus_get_unique_name(bus, &me); - if (r < 0) - return r; - - if (streq_ptr(sd_bus_message_get_sender(message), me)) - return 0; - } - /* only accept org.freedesktop.systemd1.Agent from UID=0 */ r = sd_bus_query_sender_creds(message, SD_BUS_CREDS_EUID, &creds); if (r < 0) @@ -110,16 +124,6 @@ static int signal_agent_released(sd_bus_message *message, void *userdata, sd_bus } manager_notify_cgroup_empty(m, cgroup); - - /* if running as system-instance, forward under our name */ - if (MANAGER_IS_SYSTEM(m) && m->system_bus) { - r = sd_bus_message_rewind(message, 1); - if (r >= 0) - r = sd_bus_send(m->system_bus, message, NULL); - if (r < 0) - log_warning_errno(r, "Failed to forward Released message: %m"); - } - return 0; } @@ -690,25 +694,6 @@ static int bus_on_connection(sd_event_source *s, int fd, uint32_t revents, void return 0; } - if (MANAGER_IS_SYSTEM(m)) { - /* When we run as system instance we get the Released - * signal via a direct connection */ - - r = sd_bus_add_match( - bus, - NULL, - "type='signal'," - "interface='org.freedesktop.systemd1.Agent'," - "member='Released'," - "path='/org/freedesktop/systemd1/agent'", - signal_agent_released, m); - - if (r < 0) { - log_warning_errno(r, "Failed to register Released match on new connection bus: %m"); - return 0; - } - } - r = bus_setup_disconnected_match(m, bus); if (r < 0) return 0; @@ -906,8 +891,8 @@ static int bus_setup_system(Manager *m, sd_bus *bus) { assert(m); assert(bus); - /* On kdbus or if we are a user instance we get the Released message via the system bus */ - if (MANAGER_IS_USER(m) || m->kdbus_fd >= 0) { + /* if we are a user instance we get the Released message via the system bus */ + if (MANAGER_IS_USER(m)) { r = sd_bus_add_match( bus, NULL, diff --git a/src/core/dbus.h b/src/core/dbus.h index e16a84fbb8..6baaffbd75 100644 --- a/src/core/dbus.h +++ b/src/core/dbus.h @@ -40,3 +40,5 @@ int bus_verify_manage_units_async(Manager *m, sd_bus_message *call, sd_bus_error int bus_verify_manage_unit_files_async(Manager *m, sd_bus_message *call, sd_bus_error *error); int bus_verify_reload_daemon_async(Manager *m, sd_bus_message *call, sd_bus_error *error); int bus_verify_set_environment_async(Manager *m, sd_bus_message *call, sd_bus_error *error); + +int bus_forward_agent_released(Manager *m, const char *path); diff --git a/src/core/manager.c b/src/core/manager.c index bd00c224f4..17b940c11a 100644 --- a/src/core/manager.c +++ b/src/core/manager.c @@ -87,6 +87,7 @@ #include "watchdog.h" #define NOTIFY_RCVBUF_SIZE (8*1024*1024) +#define CGROUPS_AGENT_RCVBUF_SIZE (8*1024*1024) /* Initial delay and the interval for printing status messages about running jobs */ #define JOBS_IN_PROGRESS_WAIT_USEC (5*USEC_PER_SEC) @@ -94,6 +95,7 @@ #define JOBS_IN_PROGRESS_PERIOD_DIVISOR 3 static int manager_dispatch_notify_fd(sd_event_source *source, int fd, uint32_t revents, void *userdata); +static int manager_dispatch_cgroups_agent_fd(sd_event_source *source, int fd, uint32_t revents, void *userdata); static int manager_dispatch_signal_fd(sd_event_source *source, int fd, uint32_t revents, void *userdata); static int manager_dispatch_time_change_fd(sd_event_source *source, int fd, uint32_t revents, void *userdata); static int manager_dispatch_idle_pipe_fd(sd_event_source *source, int fd, uint32_t revents, void *userdata); @@ -484,11 +486,11 @@ static int manager_setup_signals(Manager *m) { (void) sd_event_source_set_description(m->signal_event_source, "manager-signal"); - /* Process signals a bit earlier than the rest of things, but - * later than notify_fd processing, so that the notify - * processing can still figure out to which process/service a - * message belongs, before we reap the process. */ - r = sd_event_source_set_priority(m->signal_event_source, SD_EVENT_PRIORITY_NORMAL-5); + /* Process signals a bit earlier than the rest of things, but later than notify_fd processing, so that the + * notify processing can still figure out to which process/service a message belongs, before we reap the + * process. Also, process this before handling cgroup notifications, so that we always collect child exit + * status information before detecting that there's no process in a cgroup. */ + r = sd_event_source_set_priority(m->signal_event_source, SD_EVENT_PRIORITY_NORMAL-6); if (r < 0) return r; @@ -581,12 +583,12 @@ int manager_new(UnitFileScope scope, bool test_run, Manager **_m) { m->idle_pipe[0] = m->idle_pipe[1] = m->idle_pipe[2] = m->idle_pipe[3] = -1; - m->pin_cgroupfs_fd = m->notify_fd = m->signal_fd = m->time_change_fd = - m->dev_autofs_fd = m->private_listen_fd = m->kdbus_fd = m->cgroup_inotify_fd = -1; + m->pin_cgroupfs_fd = m->notify_fd = m->cgroups_agent_fd = m->signal_fd = m->time_change_fd = + m->dev_autofs_fd = m->private_listen_fd = m->kdbus_fd = m->cgroup_inotify_fd = + m->ask_password_inotify_fd = -1; m->current_job_id = 1; /* start as id #1, so that we can leave #0 around as "null-like" value */ - m->ask_password_inotify_fd = -1; m->have_ask_password = -EINVAL; /* we don't know */ m->first_boot = -1; @@ -722,8 +724,8 @@ static int manager_setup_notify(Manager *m) { if (r < 0) return log_error_errno(r, "Failed to allocate notify event source: %m"); - /* Process signals a bit earlier than SIGCHLD, so that we can - * still identify to which service an exit message belongs */ + /* Process notification messages a bit earlier than SIGCHLD, so that we can still identify to which + * service an exit message belongs. */ r = sd_event_source_set_priority(m->notify_event_source, SD_EVENT_PRIORITY_NORMAL-7); if (r < 0) return log_error_errno(r, "Failed to set priority of notify event source: %m"); @@ -734,6 +736,79 @@ static int manager_setup_notify(Manager *m) { return 0; } +static int manager_setup_cgroups_agent(Manager *m) { + + static const union sockaddr_union sa = { + .un.sun_family = AF_UNIX, + .un.sun_path = "/run/systemd/cgroups-agent", + }; + int r; + + /* This creates a listening socket we receive cgroups agent messages on. We do not use D-Bus for delivering + * these messages from the cgroups agent binary to PID 1, as the cgroups agent binary is very short-living, and + * each instance of it needs a new D-Bus connection. Since D-Bus connections are SOCK_STREAM/AF_UNIX, on + * overloaded systems the backlog of the D-Bus socket becomes relevant, as not more than the configured number + * of D-Bus connections may be queued until the kernel will start dropping further incoming connections, + * possibly resulting in lost cgroups agent messages. To avoid this, we'll use a private SOCK_DGRAM/AF_UNIX + * socket, where no backlog is relevant as communication may take place without an actual connect() cycle, and + * we thus won't lose messages. + * + * Note that PID 1 will forward the agent message to system bus, so that the user systemd instance may listen + * to it. The system instance hence listens on this special socket, but the user instances listen on the system + * bus for these messages. */ + + if (m->test_run) + return 0; + + if (!MANAGER_IS_SYSTEM(m)) + return 0; + + if (cg_unified() > 0) /* We don't need this anymore on the unified hierarchy */ + return 0; + + if (m->cgroups_agent_fd < 0) { + _cleanup_close_ int fd = -1; + + /* First free all secondary fields */ + m->cgroups_agent_event_source = sd_event_source_unref(m->cgroups_agent_event_source); + + fd = socket(AF_UNIX, SOCK_DGRAM|SOCK_CLOEXEC|SOCK_NONBLOCK, 0); + if (fd < 0) + return log_error_errno(errno, "Failed to allocate cgroups agent socket: %m"); + + fd_inc_rcvbuf(fd, CGROUPS_AGENT_RCVBUF_SIZE); + + (void) unlink(sa.un.sun_path); + + /* Only allow root to connect to this socket */ + RUN_WITH_UMASK(0077) + r = bind(fd, &sa.sa, offsetof(struct sockaddr_un, sun_path) + strlen(sa.un.sun_path)); + if (r < 0) + return log_error_errno(errno, "bind(%s) failed: %m", sa.un.sun_path); + + m->cgroups_agent_fd = fd; + fd = -1; + } + + if (!m->cgroups_agent_event_source) { + r = sd_event_add_io(m->event, &m->cgroups_agent_event_source, m->cgroups_agent_fd, EPOLLIN, manager_dispatch_cgroups_agent_fd, m); + if (r < 0) + return log_error_errno(r, "Failed to allocate cgroups agent event source: %m"); + + /* Process cgroups notifications early, but after having processed service notification messages or + * SIGCHLD signals, so that a cgroup running empty is always just the last safety net of notification, + * and we collected the metadata the notification and SIGCHLD stuff offers first. Also see handling of + * cgroup inotify for the unified cgroup stuff. */ + r = sd_event_source_set_priority(m->cgroups_agent_event_source, SD_EVENT_PRIORITY_NORMAL-5); + if (r < 0) + return log_error_errno(r, "Failed to set priority of cgroups agent event source: %m"); + + (void) sd_event_source_set_description(m->cgroups_agent_event_source, "manager-cgroups-agent"); + } + + return 0; +} + static int manager_setup_kdbus(Manager *m) { _cleanup_free_ char *p = NULL; @@ -944,12 +1019,14 @@ Manager* manager_free(Manager *m) { sd_event_source_unref(m->signal_event_source); sd_event_source_unref(m->notify_event_source); + sd_event_source_unref(m->cgroups_agent_event_source); sd_event_source_unref(m->time_change_event_source); sd_event_source_unref(m->jobs_in_progress_event_source); sd_event_source_unref(m->run_queue_event_source); safe_close(m->signal_fd); safe_close(m->notify_fd); + safe_close(m->cgroups_agent_fd); safe_close(m->time_change_fd); safe_close(m->kdbus_fd); @@ -1142,6 +1219,10 @@ int manager_startup(Manager *m, FILE *serialization, FDSet *fds) { if (q < 0 && r == 0) r = q; + q = manager_setup_cgroups_agent(m); + if (q < 0 && r == 0) + r = q; + /* We might have deserialized the kdbus control fd, but if we * didn't, then let's create the bus now. */ manager_setup_kdbus(m); @@ -1479,6 +1560,35 @@ static unsigned manager_dispatch_dbus_queue(Manager *m) { return n; } +static int manager_dispatch_cgroups_agent_fd(sd_event_source *source, int fd, uint32_t revents, void *userdata) { + Manager *m = userdata; + char buf[PATH_MAX+1]; + ssize_t n; + + n = recv(fd, buf, sizeof(buf), 0); + if (n < 0) + return log_error_errno(errno, "Failed to read cgroups agent message: %m"); + if (n == 0) { + log_error("Got zero-length cgroups agent message, ignoring."); + return 0; + } + if ((size_t) n >= sizeof(buf)) { + log_error("Got overly long cgroups agent message, ignoring."); + return 0; + } + + if (memchr(buf, 0, n)) { + log_error("Got cgroups agent message with embedded NUL byte, ignoring."); + return 0; + } + buf[n] = 0; + + manager_notify_cgroup_empty(m, buf); + bus_forward_agent_released(m, buf); + + return 0; +} + static void manager_invoke_notify_message(Manager *m, Unit *u, pid_t pid, const char *buf, size_t n, FDSet *fds) { _cleanup_strv_free_ char **tags = NULL; @@ -2265,6 +2375,16 @@ int manager_serialize(Manager *m, FILE *f, FDSet *fds, bool switching_root) { fprintf(f, "notify-socket=%s\n", m->notify_socket); } + if (m->cgroups_agent_fd >= 0) { + int copy; + + copy = fdset_put_dup(fds, m->cgroups_agent_fd); + if (copy < 0) + return copy; + + fprintf(f, "cgroups-agent-fd=%i\n", copy); + } + if (m->kdbus_fd >= 0) { int copy; @@ -2432,6 +2552,17 @@ int manager_deserialize(Manager *m, FILE *f, FDSet *fds) { free(m->notify_socket); m->notify_socket = n; + } else if (startswith(l, "cgroups-agent-fd=")) { + int fd; + + if (safe_atoi(l + 17, &fd) < 0 || fd < 0 || !fdset_contains(fds, fd)) + log_debug("Failed to parse cgroups agent fd: %s", l + 10); + else { + m->cgroups_agent_event_source = sd_event_source_unref(m->cgroups_agent_event_source); + safe_close(m->cgroups_agent_fd); + m->cgroups_agent_fd = fdset_remove(fds, fd); + } + } else if (startswith(l, "kdbus-fd=")) { int fd; @@ -2552,6 +2683,10 @@ int manager_reload(Manager *m) { if (q < 0 && r >= 0) r = q; + q = manager_setup_cgroups_agent(m); + if (q < 0 && r >= 0) + r = q; + /* Third, fire things up! */ manager_coldplug(m); diff --git a/src/core/manager.h b/src/core/manager.h index 17f84e6963..4bccca75cb 100644 --- a/src/core/manager.h +++ b/src/core/manager.h @@ -132,6 +132,9 @@ struct Manager { int notify_fd; sd_event_source *notify_event_source; + int cgroups_agent_fd; + sd_event_source *cgroups_agent_event_source; + int signal_fd; sd_event_source *signal_event_source; -- cgit v1.2.3-54-g00ecf From fc2fffe7706ef269005bf4eef56570346c9ca3da Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 5 May 2016 22:24:36 +0200 Subject: tree-wide: introduce new SOCKADDR_UN_LEN() macro, and use it everywhere The macro determines the right length of a AF_UNIX "struct sockaddr_un" to pass to connect() or bind(). It automatically figures out if the socket refers to an abstract namespace socket, or a socket in the file system, and properly handles the full length of the path field. This macro is not only safer, but also simpler to use, than the usual offsetof() + strlen() logic. --- src/basic/log.c | 6 +++--- src/basic/socket-util.h | 11 +++++++++++ src/cgroups-agent/cgroups-agent.c | 2 +- src/core/dbus.c | 2 +- src/core/execute.c | 2 +- src/core/manager.c | 11 +++++------ src/coredump/coredump.c | 2 +- src/fsck/fsck.c | 2 +- src/import/importd.c | 2 +- src/journal/journal-send.c | 14 +++++++------- src/journal/journald-native.c | 14 +++++++------- src/journal/journald-server.c | 2 +- src/journal/journald-stream.c | 13 ++++++------- src/journal/journald-syslog.c | 18 +++++++++--------- src/libsystemd/sd-bus/sd-bus.c | 2 +- src/libsystemd/sd-daemon/sd-daemon.c | 4 +--- src/login/pam_systemd.c | 2 +- src/reply-password/reply-password.c | 8 +++----- src/shared/ask-password-api.c | 2 +- src/socket-proxy/socket-proxyd.c | 17 ++++------------- src/test/test-socket-util.c | 17 +++++++++++++++++ src/tty-ask-password-agent/tty-ask-password-agent.c | 7 +++---- src/udev/udev-ctrl.c | 2 +- 23 files changed, 87 insertions(+), 75 deletions(-) (limited to 'src/core') diff --git a/src/basic/log.c b/src/basic/log.c index d89e6f7274..3ea643b6e6 100644 --- a/src/basic/log.c +++ b/src/basic/log.c @@ -165,7 +165,7 @@ static int log_open_syslog(void) { goto fail; } - if (connect(syslog_fd, &sa.sa, offsetof(struct sockaddr_un, sun_path) + strlen(sa.un.sun_path)) < 0) { + if (connect(syslog_fd, &sa.sa, SOCKADDR_UN_LEN(sa.un)) < 0) { safe_close(syslog_fd); /* Some legacy syslog systems still use stream @@ -177,7 +177,7 @@ static int log_open_syslog(void) { goto fail; } - if (connect(syslog_fd, &sa.sa, offsetof(struct sockaddr_un, sun_path) + strlen(sa.un.sun_path)) < 0) { + if (connect(syslog_fd, &sa.sa, SOCKADDR_UN_LEN(sa.un)) < 0) { r = -errno; goto fail; } @@ -215,7 +215,7 @@ static int log_open_journal(void) { goto fail; } - if (connect(journal_fd, &sa.sa, offsetof(struct sockaddr_un, sun_path) + strlen(sa.un.sun_path)) < 0) { + if (connect(journal_fd, &sa.sa, SOCKADDR_UN_LEN(sa.un)) < 0) { r = -errno; goto fail; } diff --git a/src/basic/socket-util.h b/src/basic/socket-util.h index d17a2f35f8..daa4b24a37 100644 --- a/src/basic/socket-util.h +++ b/src/basic/socket-util.h @@ -137,3 +137,14 @@ ssize_t next_datagram_size_fd(int fd); #define CMSG_FOREACH(cmsg, mh) \ for ((cmsg) = CMSG_FIRSTHDR(mh); (cmsg); (cmsg) = CMSG_NXTHDR((mh), (cmsg))) + +/* Covers only file system and abstract AF_UNIX socket addresses, but not unnamed socket addresses. */ +#define SOCKADDR_UN_LEN(sa) \ + ({ \ + const struct sockaddr_un *_sa = &(sa); \ + assert(_sa->sun_family == AF_UNIX); \ + offsetof(struct sockaddr_un, sun_path) + \ + (_sa->sun_path[0] == 0 ? \ + 1 + strnlen(_sa->sun_path+1, sizeof(_sa->sun_path)-1) : \ + strnlen(_sa->sun_path, sizeof(_sa->sun_path))); \ + }) diff --git a/src/cgroups-agent/cgroups-agent.c b/src/cgroups-agent/cgroups-agent.c index 333ce110d3..d7c722ac3d 100644 --- a/src/cgroups-agent/cgroups-agent.c +++ b/src/cgroups-agent/cgroups-agent.c @@ -52,7 +52,7 @@ int main(int argc, char *argv[]) { l = strlen(argv[1]); - n = sendto(fd, argv[1], l, 0, &sa.sa, offsetof(struct sockaddr_un, sun_path) + strlen(sa.un.sun_path)); + n = sendto(fd, argv[1], l, 0, &sa.sa, SOCKADDR_UN_LEN(sa.un)); if (n < 0) { log_debug_errno(errno, "Failed to send cgroups agent message: %m"); return EXIT_FAILURE; diff --git a/src/core/dbus.c b/src/core/dbus.c index c8375a0475..3422a02d68 100644 --- a/src/core/dbus.c +++ b/src/core/dbus.c @@ -975,7 +975,7 @@ static int bus_init_private(Manager *m) { return 0; strcpy(sa.un.sun_path, "/run/systemd/private"); - salen = offsetof(union sockaddr_union, un.sun_path) + strlen("/run/systemd/private"); + salen = SOCKADDR_UN_LEN(sa.un); } else { size_t left = sizeof(sa.un.sun_path); char *p = sa.un.sun_path; diff --git a/src/core/execute.c b/src/core/execute.c index ac2ac39892..5eb3f13695 100644 --- a/src/core/execute.c +++ b/src/core/execute.c @@ -271,7 +271,7 @@ static int connect_journal_socket(int fd, uid_t uid, gid_t gid) { } } - r = connect(fd, &sa.sa, offsetof(struct sockaddr_un, sun_path) + strlen(sa.un.sun_path)); + r = connect(fd, &sa.sa, SOCKADDR_UN_LEN(sa.un)); if (r < 0) r = -errno; diff --git a/src/core/manager.c b/src/core/manager.c index 17b940c11a..e192cd475d 100644 --- a/src/core/manager.c +++ b/src/core/manager.c @@ -705,7 +705,7 @@ static int manager_setup_notify(Manager *m) { (void) unlink(m->notify_socket); strncpy(sa.un.sun_path, m->notify_socket, sizeof(sa.un.sun_path)-1); - r = bind(fd, &sa.sa, offsetof(struct sockaddr_un, sun_path) + strlen(sa.un.sun_path)); + r = bind(fd, &sa.sa, SOCKADDR_UN_LEN(sa.un)); if (r < 0) return log_error_errno(errno, "bind(%s) failed: %m", sa.un.sun_path); @@ -782,7 +782,7 @@ static int manager_setup_cgroups_agent(Manager *m) { /* Only allow root to connect to this socket */ RUN_WITH_UMASK(0077) - r = bind(fd, &sa.sa, offsetof(struct sockaddr_un, sun_path) + strlen(sa.un.sun_path)); + r = bind(fd, &sa.sa, SOCKADDR_UN_LEN(sa.un)); if (r < 0) return log_error_errno(errno, "bind(%s) failed: %m", sa.un.sun_path); @@ -2245,11 +2245,10 @@ void manager_send_unit_audit(Manager *m, Unit *u, int type, bool success) { } void manager_send_unit_plymouth(Manager *m, Unit *u) { - union sockaddr_union sa = PLYMOUTH_SOCKET; - - int n = 0; + static const union sockaddr_union sa = PLYMOUTH_SOCKET; _cleanup_free_ char *message = NULL; _cleanup_close_ int fd = -1; + int n = 0; /* Don't generate plymouth events if the service was already * started and we're just deserializing */ @@ -2275,7 +2274,7 @@ void manager_send_unit_plymouth(Manager *m, Unit *u) { return; } - if (connect(fd, &sa.sa, offsetof(struct sockaddr_un, sun_path) + 1 + strlen(sa.un.sun_path+1)) < 0) { + if (connect(fd, &sa.sa, SOCKADDR_UN_LEN(sa.un)) < 0) { if (!IN_SET(errno, EPIPE, EAGAIN, ENOENT, ECONNREFUSED, ECONNRESET, ECONNABORTED)) log_error_errno(errno, "connect() failed: %m"); diff --git a/src/coredump/coredump.c b/src/coredump/coredump.c index 41fc1993d5..01fdcfa909 100644 --- a/src/coredump/coredump.c +++ b/src/coredump/coredump.c @@ -847,7 +847,7 @@ static int send_iovec(const struct iovec iovec[], size_t n_iovec, int input_fd) if (fd < 0) return log_error_errno(errno, "Failed to create coredump socket: %m"); - if (connect(fd, &sa.sa, offsetof(union sockaddr_union, un.sun_path) + strlen(sa.un.sun_path)) < 0) + if (connect(fd, &sa.sa, SOCKADDR_UN_LEN(sa.un)) < 0) return log_error_errno(errno, "Failed to connect to coredump service: %m"); for (i = 0; i < n_iovec; i++) { diff --git a/src/fsck/fsck.c b/src/fsck/fsck.c index 6f56066da8..d32e1d923e 100644 --- a/src/fsck/fsck.c +++ b/src/fsck/fsck.c @@ -262,7 +262,7 @@ static int fsck_progress_socket(void) { if (fd < 0) return log_warning_errno(errno, "socket(): %m"); - if (connect(fd, &sa.sa, offsetof(struct sockaddr_un, sun_path) + strlen(sa.un.sun_path)) < 0) { + if (connect(fd, &sa.sa, SOCKADDR_UN_LEN(sa.un)) < 0) { r = log_full_errno(errno == ECONNREFUSED || errno == ENOENT ? LOG_DEBUG : LOG_WARNING, errno, "Failed to connect to progress socket %s, ignoring: %m", sa.un.sun_path); safe_close(fd); diff --git a/src/import/importd.c b/src/import/importd.c index d2a5867a6e..956a82945c 100644 --- a/src/import/importd.c +++ b/src/import/importd.c @@ -677,7 +677,7 @@ static int manager_new(Manager **ret) { (void) mkdir_parents_label(sa.un.sun_path, 0755); (void) unlink(sa.un.sun_path); - if (bind(m->notify_fd, &sa.sa, offsetof(union sockaddr_union, un.sun_path) + strlen(sa.un.sun_path)) < 0) + if (bind(m->notify_fd, &sa.sa, SOCKADDR_UN_LEN(sa.un)) < 0) return -errno; if (setsockopt(m->notify_fd, SOL_SOCKET, SO_PASSCRED, &one, sizeof(one)) < 0) diff --git a/src/journal/journal-send.c b/src/journal/journal-send.c index f0959b6237..5e8a3e3200 100644 --- a/src/journal/journal-send.c +++ b/src/journal/journal-send.c @@ -208,13 +208,13 @@ _public_ int sd_journal_sendv(const struct iovec *iov, int n) { struct iovec *w; uint64_t *l; int i, j = 0; - struct sockaddr_un sa = { - .sun_family = AF_UNIX, - .sun_path = "/run/systemd/journal/socket", + static const union sockaddr_union sa = { + .un.sun_family = AF_UNIX, + .un.sun_path = "/run/systemd/journal/socket", }; struct msghdr mh = { - .msg_name = &sa, - .msg_namelen = offsetof(struct sockaddr_un, sun_path) + strlen(sa.sun_path), + .msg_name = (struct sockaddr*) &sa.sa, + .msg_namelen = SOCKADDR_UN_LEN(sa.un), }; ssize_t k; bool have_syslog_identifier = false; @@ -392,7 +392,7 @@ _public_ int sd_journal_perror(const char *message) { } _public_ int sd_journal_stream_fd(const char *identifier, int priority, int level_prefix) { - union sockaddr_union sa = { + static const union sockaddr_union sa = { .un.sun_family = AF_UNIX, .un.sun_path = "/run/systemd/journal/stdout", }; @@ -408,7 +408,7 @@ _public_ int sd_journal_stream_fd(const char *identifier, int priority, int leve if (fd < 0) return -errno; - r = connect(fd, &sa.sa, offsetof(union sockaddr_union, un.sun_path) + strlen(sa.un.sun_path)); + r = connect(fd, &sa.sa, SOCKADDR_UN_LEN(sa.un)); if (r < 0) return -errno; diff --git a/src/journal/journald-native.c b/src/journal/journald-native.c index a445291a5e..0a1ce205c2 100644 --- a/src/journal/journald-native.c +++ b/src/journal/journald-native.c @@ -448,24 +448,24 @@ void server_process_native_file( } int server_open_native_socket(Server*s) { + + static const union sockaddr_union sa = { + .un.sun_family = AF_UNIX, + .un.sun_path = "/run/systemd/journal/socket", + }; static const int one = 1; int r; assert(s); if (s->native_fd < 0) { - union sockaddr_union sa = { - .un.sun_family = AF_UNIX, - .un.sun_path = "/run/systemd/journal/socket", - }; - s->native_fd = socket(AF_UNIX, SOCK_DGRAM|SOCK_CLOEXEC|SOCK_NONBLOCK, 0); if (s->native_fd < 0) return log_error_errno(errno, "socket() failed: %m"); - unlink(sa.un.sun_path); + (void) unlink(sa.un.sun_path); - r = bind(s->native_fd, &sa.sa, offsetof(union sockaddr_union, un.sun_path) + strlen(sa.un.sun_path)); + r = bind(s->native_fd, &sa.sa, SOCKADDR_UN_LEN(sa.un)); if (r < 0) return log_error_errno(errno, "bind(%s) failed: %m", sa.un.sun_path); diff --git a/src/journal/journald-server.c b/src/journal/journald-server.c index e14d0ad980..8f82d2a838 100644 --- a/src/journal/journald-server.c +++ b/src/journal/journald-server.c @@ -1696,7 +1696,7 @@ static int server_connect_notify(Server *s) { if (sa.un.sun_path[0] == '@') sa.un.sun_path[0] = 0; - r = connect(s->notify_fd, &sa.sa, offsetof(struct sockaddr_un, sun_path) + strlen(e)); + r = connect(s->notify_fd, &sa.sa, SOCKADDR_UN_LEN(sa.un)); if (r < 0) return log_error_errno(errno, "Failed to connect to notify socket: %m"); diff --git a/src/journal/journald-stream.c b/src/journal/journald-stream.c index 59352bcb3f..4ad16ee41c 100644 --- a/src/journal/journald-stream.c +++ b/src/journal/journald-stream.c @@ -700,23 +700,22 @@ fail: } int server_open_stdout_socket(Server *s) { + static const union sockaddr_union sa = { + .un.sun_family = AF_UNIX, + .un.sun_path = "/run/systemd/journal/stdout", + }; int r; assert(s); if (s->stdout_fd < 0) { - union sockaddr_union sa = { - .un.sun_family = AF_UNIX, - .un.sun_path = "/run/systemd/journal/stdout", - }; - s->stdout_fd = socket(AF_UNIX, SOCK_STREAM|SOCK_CLOEXEC|SOCK_NONBLOCK, 0); if (s->stdout_fd < 0) return log_error_errno(errno, "socket() failed: %m"); - unlink(sa.un.sun_path); + (void) unlink(sa.un.sun_path); - r = bind(s->stdout_fd, &sa.sa, offsetof(union sockaddr_union, un.sun_path) + strlen(sa.un.sun_path)); + r = bind(s->stdout_fd, &sa.sa, SOCKADDR_UN_LEN(sa.un)); if (r < 0) return log_error_errno(errno, "bind(%s) failed: %m", sa.un.sun_path); diff --git a/src/journal/journald-syslog.c b/src/journal/journald-syslog.c index 5153fd0cce..ead47887d8 100644 --- a/src/journal/journald-syslog.c +++ b/src/journal/journald-syslog.c @@ -52,8 +52,7 @@ static void forward_syslog_iovec(Server *s, const struct iovec *iovec, unsigned .msg_iov = (struct iovec *) iovec, .msg_iovlen = n_iovec, .msg_name = (struct sockaddr*) &sa.sa, - .msg_namelen = offsetof(union sockaddr_union, un.sun_path) - + strlen("/run/systemd/journal/syslog"), + .msg_namelen = SOCKADDR_UN_LEN(sa.un), }; struct cmsghdr *cmsg; union { @@ -383,24 +382,24 @@ void server_process_syslog_message( } int server_open_syslog_socket(Server *s) { + + static const union sockaddr_union sa = { + .un.sun_family = AF_UNIX, + .un.sun_path = "/run/systemd/journal/dev-log", + }; static const int one = 1; int r; assert(s); if (s->syslog_fd < 0) { - static const union sockaddr_union sa = { - .un.sun_family = AF_UNIX, - .un.sun_path = "/run/systemd/journal/dev-log", - }; - s->syslog_fd = socket(AF_UNIX, SOCK_DGRAM|SOCK_CLOEXEC|SOCK_NONBLOCK, 0); if (s->syslog_fd < 0) return log_error_errno(errno, "socket() failed: %m"); - unlink(sa.un.sun_path); + (void) unlink(sa.un.sun_path); - r = bind(s->syslog_fd, &sa.sa, offsetof(union sockaddr_union, un.sun_path) + strlen(sa.un.sun_path)); + r = bind(s->syslog_fd, &sa.sa, SOCKADDR_UN_LEN(sa.un)); if (r < 0) return log_error_errno(errno, "bind(%s) failed: %m", sa.un.sun_path); @@ -437,6 +436,7 @@ int server_open_syslog_socket(Server *s) { void server_maybe_warn_forward_syslog_missed(Server *s) { usec_t n; + assert(s); if (s->n_forward_syslog_missed <= 0) diff --git a/src/libsystemd/sd-bus/sd-bus.c b/src/libsystemd/sd-bus/sd-bus.c index 04da94e7e3..ed5f94e136 100644 --- a/src/libsystemd/sd-bus/sd-bus.c +++ b/src/libsystemd/sd-bus/sd-bus.c @@ -836,7 +836,7 @@ static int parse_container_unix_address(sd_bus *b, const char **p, char **guid) b->sockaddr.un.sun_family = AF_UNIX; strncpy(b->sockaddr.un.sun_path, "/var/run/dbus/system_bus_socket", sizeof(b->sockaddr.un.sun_path)); - b->sockaddr_size = offsetof(struct sockaddr_un, sun_path) + strlen("/var/run/dbus/system_bus_socket"); + b->sockaddr_size = SOCKADDR_UN_LEN(b->sockaddr.un); return 0; } diff --git a/src/libsystemd/sd-daemon/sd-daemon.c b/src/libsystemd/sd-daemon/sd-daemon.c index bd1c7f15ff..4da9dbfd63 100644 --- a/src/libsystemd/sd-daemon/sd-daemon.c +++ b/src/libsystemd/sd-daemon/sd-daemon.c @@ -458,9 +458,7 @@ _public_ int sd_pid_notify_with_fds(pid_t pid, int unset_environment, const char if (sockaddr.un.sun_path[0] == '@') sockaddr.un.sun_path[0] = 0; - msghdr.msg_namelen = offsetof(struct sockaddr_un, sun_path) + strlen(e); - if (msghdr.msg_namelen > sizeof(struct sockaddr_un)) - msghdr.msg_namelen = sizeof(struct sockaddr_un); + msghdr.msg_namelen = SOCKADDR_UN_LEN(sockaddr.un); have_pid = pid != 0 && pid != getpid(); diff --git a/src/login/pam_systemd.c b/src/login/pam_systemd.c index 40e246bb06..98dc201340 100644 --- a/src/login/pam_systemd.c +++ b/src/login/pam_systemd.c @@ -150,7 +150,7 @@ static int get_seat_from_display(const char *display, const char **seat, uint32_ if (fd < 0) return -errno; - if (connect(fd, &sa.sa, offsetof(struct sockaddr_un, sun_path) + strlen(sa.un.sun_path)) < 0) + if (connect(fd, &sa.sa, SOCKADDR_UN_LEN(sa.un)) < 0) return -errno; r = getpeercred(fd, &ucred); diff --git a/src/reply-password/reply-password.c b/src/reply-password/reply-password.c index e291758969..17eab9772e 100644 --- a/src/reply-password/reply-password.c +++ b/src/reply-password/reply-password.c @@ -26,14 +26,12 @@ #include "fd-util.h" #include "log.h" #include "macro.h" +#include "socket-util.h" #include "string-util.h" #include "util.h" static int send_on_socket(int fd, const char *socket_name, const void *packet, size_t size) { - union { - struct sockaddr sa; - struct sockaddr_un un; - } sa = { + union sockaddr_union sa = { .un.sun_family = AF_UNIX, }; @@ -43,7 +41,7 @@ static int send_on_socket(int fd, const char *socket_name, const void *packet, s strncpy(sa.un.sun_path, socket_name, sizeof(sa.un.sun_path)); - if (sendto(fd, packet, size, MSG_NOSIGNAL, &sa.sa, offsetof(struct sockaddr_un, sun_path) + strlen(socket_name)) < 0) + if (sendto(fd, packet, size, MSG_NOSIGNAL, &sa.sa, SOCKADDR_UN_LEN(sa.un)) < 0) return log_error_errno(errno, "Failed to send: %m"); return 0; diff --git a/src/shared/ask-password-api.c b/src/shared/ask-password-api.c index 6805873f9e..4a4bd8d3b8 100644 --- a/src/shared/ask-password-api.c +++ b/src/shared/ask-password-api.c @@ -431,7 +431,7 @@ static int create_socket(char **name) { snprintf(sa.un.sun_path, sizeof(sa.un.sun_path)-1, "/run/systemd/ask-password/sck.%" PRIx64, random_u64()); RUN_WITH_UMASK(0177) { - if (bind(fd, &sa.sa, offsetof(struct sockaddr_un, sun_path) + strlen(sa.un.sun_path)) < 0) + if (bind(fd, &sa.sa, SOCKADDR_UN_LEN(sa.un)) < 0) return -errno; } diff --git a/src/socket-proxy/socket-proxyd.c b/src/socket-proxy/socket-proxyd.c index 1157a0c72e..52b4db8875 100644 --- a/src/socket-proxy/socket-proxyd.c +++ b/src/socket-proxy/socket-proxyd.c @@ -400,28 +400,19 @@ static int resolve_remote(Connection *c) { union sockaddr_union sa = {}; const char *node, *service; - socklen_t salen; int r; if (path_is_absolute(arg_remote_host)) { sa.un.sun_family = AF_UNIX; - strncpy(sa.un.sun_path, arg_remote_host, sizeof(sa.un.sun_path)-1); - sa.un.sun_path[sizeof(sa.un.sun_path)-1] = 0; - - salen = offsetof(union sockaddr_union, un.sun_path) + strlen(sa.un.sun_path); - - return connection_start(c, &sa.sa, salen); + strncpy(sa.un.sun_path, arg_remote_host, sizeof(sa.un.sun_path)); + return connection_start(c, &sa.sa, SOCKADDR_UN_LEN(sa.un)); } if (arg_remote_host[0] == '@') { sa.un.sun_family = AF_UNIX; sa.un.sun_path[0] = 0; - strncpy(sa.un.sun_path+1, arg_remote_host+1, sizeof(sa.un.sun_path)-2); - sa.un.sun_path[sizeof(sa.un.sun_path)-1] = 0; - - salen = offsetof(union sockaddr_union, un.sun_path) + 1 + strlen(sa.un.sun_path + 1); - - return connection_start(c, &sa.sa, salen); + strncpy(sa.un.sun_path+1, arg_remote_host+1, sizeof(sa.un.sun_path)-1); + return connection_start(c, &sa.sa, SOCKADDR_UN_LEN(sa.un)); } service = strrchr(arg_remote_host, ':'); diff --git a/src/test/test-socket-util.c b/src/test/test-socket-util.c index 33ff3755bc..9e01f3afd4 100644 --- a/src/test/test-socket-util.c +++ b/src/test/test-socket-util.c @@ -343,6 +343,21 @@ static void test_sockaddr_equal(void) { assert_se(!sockaddr_equal(&b, &c)); } +static void test_sockaddr_un_len(void) { + static const struct sockaddr_un fs = { + .sun_family = AF_UNIX, + .sun_path = "/foo/bar/waldo", + }; + + static const struct sockaddr_un abstract = { + .sun_family = AF_UNIX, + .sun_path = "\0foobar", + }; + + assert_se(SOCKADDR_UN_LEN(fs) == offsetof(struct sockaddr_un, sun_path) + strlen(fs.sun_path)); + assert_se(SOCKADDR_UN_LEN(abstract) == offsetof(struct sockaddr_un, sun_path) + 1 + strlen(abstract.sun_path + 1)); +} + int main(int argc, char *argv[]) { log_set_max_level(LOG_DEBUG); @@ -363,5 +378,7 @@ int main(int argc, char *argv[]) { test_sockaddr_equal(); + test_sockaddr_un_len(); + return 0; } diff --git a/src/tty-ask-password-agent/tty-ask-password-agent.c b/src/tty-ask-password-agent/tty-ask-password-agent.c index c7ded451a2..ee879c7b89 100644 --- a/src/tty-ask-password-agent/tty-ask-password-agent.c +++ b/src/tty-ask-password-agent/tty-ask-password-agent.c @@ -65,8 +65,8 @@ static int ask_password_plymouth( const char *flag_file, char ***ret) { + static const union sockaddr_union sa = PLYMOUTH_SOCKET; _cleanup_close_ int fd = -1, notify = -1; - union sockaddr_union sa = PLYMOUTH_SOCKET; _cleanup_free_ char *packet = NULL; ssize_t k; int r, n; @@ -94,7 +94,7 @@ static int ask_password_plymouth( if (fd < 0) return -errno; - r = connect(fd, &sa.sa, offsetof(struct sockaddr_un, sun_path) + 1 + strlen(sa.un.sun_path+1)); + r = connect(fd, &sa.sa, SOCKADDR_UN_LEN(sa.un)); if (r < 0) return -errno; @@ -269,8 +269,7 @@ static int send_passwords(const char *socket_name, char **passwords) { strncpy(sa.un.sun_path, socket_name, sizeof(sa.un.sun_path)); - r = sendto(socket_fd, packet, packet_length, MSG_NOSIGNAL, &sa.sa, - offsetof(struct sockaddr_un, sun_path) + strlen(socket_name)); + r = sendto(socket_fd, packet, packet_length, MSG_NOSIGNAL, &sa.sa, SOCKADDR_UN_LEN(sa.un)); if (r < 0) r = log_debug_errno(errno, "sendto(): %m"); diff --git a/src/udev/udev-ctrl.c b/src/udev/udev-ctrl.c index 962de22f43..f68a09d7a8 100644 --- a/src/udev/udev-ctrl.c +++ b/src/udev/udev-ctrl.c @@ -105,7 +105,7 @@ struct udev_ctrl *udev_ctrl_new_from_fd(struct udev *udev, int fd) { uctrl->saddr.un.sun_family = AF_LOCAL; strscpy(uctrl->saddr.un.sun_path, sizeof(uctrl->saddr.un.sun_path), "/run/udev/control"); - uctrl->addrlen = offsetof(struct sockaddr_un, sun_path) + strlen(uctrl->saddr.un.sun_path); + uctrl->addrlen = SOCKADDR_UN_LEN(uctrl->saddr.un); return uctrl; } -- cgit v1.2.3-54-g00ecf