From 395745ba533ac91fe118f43ec83f13a752c0b473 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 1 Sep 2015 19:39:14 +0200 Subject: machined: call unlockpt() in container, not host It makes assumptions about the pty path, hence better call it in the container namespace rather than the host. --- src/basic/util.c | 3 +++ src/machine/machine-dbus.c | 6 ------ src/run/run.c | 6 +++--- 3 files changed, 6 insertions(+), 9 deletions(-) (limited to 'src') diff --git a/src/basic/util.c b/src/basic/util.c index 737f2a221c..f01f5f237b 100644 --- a/src/basic/util.c +++ b/src/basic/util.c @@ -6095,6 +6095,9 @@ int openpt_in_namespace(pid_t pid, int flags) { if (master < 0) _exit(EXIT_FAILURE); + if (unlockpt(master) < 0) + _exit(EXIT_FAILURE); + cmsg = CMSG_FIRSTHDR(&mh); cmsg->cmsg_level = SOL_SOCKET; cmsg->cmsg_type = SCM_RIGHTS; diff --git a/src/machine/machine-dbus.c b/src/machine/machine-dbus.c index af2b8eff06..f27b58b893 100644 --- a/src/machine/machine-dbus.c +++ b/src/machine/machine-dbus.c @@ -597,9 +597,6 @@ int bus_machine_method_open_login(sd_bus_message *message, void *userdata, sd_bu if (!p) return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "PTS name %s is invalid", pty_name); - if (unlockpt(master) < 0) - return -errno; - r = container_bus_new(m, &allocated_bus); if (r < 0) return r; @@ -701,9 +698,6 @@ int bus_machine_method_open_shell(sd_bus_message *message, void *userdata, sd_bu utmp_id = path_startswith(pty_name, "/dev/"); assert(utmp_id); - if (unlockpt(master) < 0) - return -errno; - r = container_bus_new(m, &allocated_bus); if (r < 0) return r; diff --git a/src/run/run.c b/src/run/run.c index a69560208c..657c6fcaf1 100644 --- a/src/run/run.c +++ b/src/run/run.c @@ -702,6 +702,9 @@ static int start_transient_service( if (r < 0) return log_error_errno(r, "Failed to determine tty name: %m"); + if (unlockpt(master) < 0) + return log_error_errno(errno, "Failed to unlock tty: %m"); + } else if (arg_transport == BUS_TRANSPORT_MACHINE) { _cleanup_bus_unref_ sd_bus *system_bus = NULL; const char *s; @@ -738,9 +741,6 @@ static int start_transient_service( return log_oom(); } else assert_not_reached("Can't allocate tty via ssh"); - - if (unlockpt(master) < 0) - return log_error_errno(errno, "Failed to unlock tty: %m"); } if (!arg_no_block) { -- cgit v1.2.3-54-g00ecf From a07c35c3e65c16264cb25206c2d564afdbae8a28 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 1 Sep 2015 20:12:25 +0200 Subject: machined: introduce a ptsname_namespace() call and make use of it The call is like ptsname() but does not assume the pty path was accessible in the local namespace. It uses the same internal ioctl though. --- src/basic/terminal-util.c | 19 +++++++++++++++++++ src/basic/terminal-util.h | 2 ++ src/machine/machine-dbus.c | 7 ++++--- 3 files changed, 25 insertions(+), 3 deletions(-) (limited to 'src') diff --git a/src/basic/terminal-util.c b/src/basic/terminal-util.c index cf55263bbf..c5ef5ab0d1 100644 --- a/src/basic/terminal-util.c +++ b/src/basic/terminal-util.c @@ -1074,3 +1074,22 @@ int get_ctty(pid_t pid, dev_t *_devnr, char **r) { return 0; } + +int ptsname_namespace(int pty, char **ret) { + int no = -1, r; + + /* Like ptsname(), but doesn't assume that the path is + * accessible in the local namespace. */ + + r = ioctl(pty, TIOCGPTN, &no); + if (r < 0) + return -errno; + + if (no < 0) + return -EIO; + + if (asprintf(ret, "/dev/pts/%i", no) < 0) + return -ENOMEM; + + return 0; +} diff --git a/src/basic/terminal-util.h b/src/basic/terminal-util.h index 188714f228..b9a3809a6c 100644 --- a/src/basic/terminal-util.h +++ b/src/basic/terminal-util.h @@ -107,3 +107,5 @@ int get_ctty(pid_t, dev_t *_devnr, char **r); int getttyname_malloc(int fd, char **r); int getttyname_harder(int fd, char **r); + +int ptsname_namespace(int pty, char **ret); diff --git a/src/machine/machine-dbus.c b/src/machine/machine-dbus.c index f27b58b893..a63b9785af 100644 --- a/src/machine/machine-dbus.c +++ b/src/machine/machine-dbus.c @@ -45,6 +45,7 @@ #include "formats-util.h" #include "process-util.h" #include "env-util.h" +#include "terminal-util.h" static int property_get_id( sd_bus *bus, @@ -500,7 +501,7 @@ int bus_machine_method_open_pty(sd_bus_message *message, void *userdata, sd_bus_ if (master < 0) return master; - r = ptsname_malloc(master, &pty_name); + r = ptsname_namespace(master, &pty_name); if (r < 0) return r; @@ -589,7 +590,7 @@ int bus_machine_method_open_login(sd_bus_message *message, void *userdata, sd_bu if (master < 0) return master; - r = ptsname_malloc(master, &pty_name); + r = ptsname_namespace(master, &pty_name); if (r < 0) return r; @@ -687,7 +688,7 @@ int bus_machine_method_open_shell(sd_bus_message *message, void *userdata, sd_bu if (master < 0) return master; - r = ptsname_malloc(master, &pty_name); + r = ptsname_namespace(master, &pty_name); if (r < 0) return r; -- cgit v1.2.3-54-g00ecf From 80b0d3e3116f85cab8b19caf1cfa9d56be190a2e Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 1 Sep 2015 20:36:52 +0200 Subject: sd-bus: when connecting to a kdbus container bus pass error up We rely on the correct error used when opening the kdbus device node, hence let's make sure we pass it up from the namespaced child process to the process which actually wants to connect. --- src/libsystemd/sd-bus/bus-container.c | 46 +++++++++++++++++++++++------------ 1 file changed, 31 insertions(+), 15 deletions(-) (limited to 'src') diff --git a/src/libsystemd/sd-bus/bus-container.c b/src/libsystemd/sd-bus/bus-container.c index 101e4af18d..56dc086ae2 100644 --- a/src/libsystemd/sd-bus/bus-container.c +++ b/src/libsystemd/sd-bus/bus-container.c @@ -125,15 +125,22 @@ int bus_container_connect_kernel(sd_bus *b) { struct cmsghdr cmsghdr; uint8_t buf[CMSG_SPACE(sizeof(int))]; } control = {}; + int error_buf = 0; + struct iovec iov = { + .iov_base = &error_buf, + .iov_len = sizeof(error_buf), + }; struct msghdr mh = { .msg_control = &control, .msg_controllen = sizeof(control), + .msg_iov = &iov, + .msg_iovlen = 1, }; struct cmsghdr *cmsg; pid_t child; siginfo_t si; - int r; - _cleanup_close_ int fd = -1; + int r, fd = -1; + ssize_t n; assert(b); assert(b->input_fd < 0); @@ -178,10 +185,13 @@ int bus_container_connect_kernel(sd_bus *b) { _exit(EXIT_FAILURE); if (grandchild == 0) { - fd = open(b->kernel, O_RDWR|O_NOCTTY|O_CLOEXEC); - if (fd < 0) + if (fd < 0) { + /* Try to send error up */ + error_buf = errno; + (void) write(pair[1], &error_buf, sizeof(error_buf)); _exit(EXIT_FAILURE); + } cmsg = CMSG_FIRSTHDR(&mh); cmsg->cmsg_level = SOL_SOCKET; @@ -213,20 +223,17 @@ int bus_container_connect_kernel(sd_bus *b) { if (r < 0) return r; - if (si.si_code != CLD_EXITED) - return -EIO; - - if (si.si_status != EXIT_SUCCESS) - return -EIO; - - if (recvmsg(pair[0], &mh, MSG_NOSIGNAL|MSG_CMSG_CLOEXEC) < 0) + n = recvmsg(pair[0], &mh, MSG_NOSIGNAL|MSG_CMSG_CLOEXEC); + if (n < 0) return -errno; - CMSG_FOREACH(cmsg, &mh) + CMSG_FOREACH(cmsg, &mh) { if (cmsg->cmsg_level == SOL_SOCKET && cmsg->cmsg_type == SCM_RIGHTS) { int *fds; unsigned n_fds; + assert(fd < 0); + fds = (int*) CMSG_DATA(cmsg); n_fds = (cmsg->cmsg_len - CMSG_LEN(0)) / sizeof(int); @@ -237,9 +244,18 @@ int bus_container_connect_kernel(sd_bus *b) { fd = fds[0]; } + } + + /* If there's an fd passed, we are good. */ + if (fd >= 0) { + b->input_fd = b->output_fd = fd; + return bus_kernel_take_fd(b); + } - b->input_fd = b->output_fd = fd; - fd = -1; + /* If there's an error passed, use it */ + if (n == sizeof(error_buf) && error_buf > 0) + return -error_buf; - return bus_kernel_take_fd(b); + /* Otherwise, we have no clue */ + return -EIO; } -- cgit v1.2.3-54-g00ecf From 751090cc8ae09787402d60b7080d479b3afe3b13 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 1 Sep 2015 20:38:37 +0200 Subject: sd-bus: when connecting to a container, don't fall back to host bus We should never connect to the host bus as fallback if connecting to a container failed via one method. Otherwise connecting to a dbus1 container will always result in a connection to the host. --- src/libsystemd/sd-bus/sd-bus.c | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) (limited to 'src') diff --git a/src/libsystemd/sd-bus/sd-bus.c b/src/libsystemd/sd-bus/sd-bus.c index 5285278d92..4ed508427e 100644 --- a/src/libsystemd/sd-bus/sd-bus.c +++ b/src/libsystemd/sd-bus/sd-bus.c @@ -1025,19 +1025,30 @@ static int bus_start_address(sd_bus *b) { if (b->exec_path) r = bus_socket_exec(b); + else if ((b->nspid > 0 || b->machine) && b->kernel) { r = bus_container_connect_kernel(b); if (r < 0 && !IN_SET(r, -ENOENT, -ESOCKTNOSUPPORT)) container_kdbus_available = true; - } else if (!container_kdbus_available && (b->nspid > 0 || b->machine) && b->sockaddr.sa.sa_family != AF_UNSPEC) - r = bus_container_connect_socket(b); - else if (b->kernel) { + + } else if ((b->nspid > 0 || b->machine) && b->sockaddr.sa.sa_family != AF_UNSPEC) { + if (!container_kdbus_available) + r = bus_container_connect_socket(b); + else + skipped = true; + + } else if (b->kernel) { r = bus_kernel_connect(b); if (r < 0 && !IN_SET(r, -ENOENT, -ESOCKTNOSUPPORT)) kdbus_available = true; - } else if (!kdbus_available && b->sockaddr.sa.sa_family != AF_UNSPEC) - r = bus_socket_connect(b); - else + + } else if (b->sockaddr.sa.sa_family != AF_UNSPEC) { + if (!kdbus_available) + r = bus_socket_connect(b); + else + skipped = true; + + } else skipped = true; if (!skipped) { -- cgit v1.2.3-54-g00ecf