From a119ec7c824da4a618cb3f4c6e3557c38381b674 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 9 Aug 2017 19:01:18 +0200 Subject: [PATCH 1/3] util-lib: add a new skip_dev_prefix() helper This new helper removes a leading /dev if there is one. We have code doing this all over the place, let's unify this, and correct it while we are at it, by using path_startswith() rather than startswith() to drop the prefix. --- src/basic/path-util.h | 10 ++++++++++ src/basic/terminal-util.c | 15 ++++----------- src/core/execute.c | 3 +-- src/test/test-path-util.c | 16 ++++++++++++++++ 4 files changed, 31 insertions(+), 13 deletions(-) diff --git a/src/basic/path-util.h b/src/basic/path-util.h index 26f165fc38..ea00955818 100644 --- a/src/basic/path-util.h +++ b/src/basic/path-util.h @@ -143,3 +143,13 @@ bool is_deviceallow_pattern(const char *path); int systemd_installation_has_version(const char *root, unsigned minimal_version); bool dot_or_dot_dot(const char *path); + +static inline const char *skip_dev_prefix(const char *p) { + const char *e; + + /* Drop any /dev prefix if there is any */ + + e = path_startswith(p, "/dev/"); + + return e ?: p; +} diff --git a/src/basic/terminal-util.c b/src/basic/terminal-util.c index 8a0419df75..c570c2e356 100644 --- a/src/basic/terminal-util.c +++ b/src/basic/terminal-util.c @@ -649,10 +649,7 @@ bool tty_is_vc(const char *tty) { bool tty_is_console(const char *tty) { assert(tty); - if (startswith(tty, "/dev/")) - tty += 5; - - return streq(tty, "console"); + return streq(skip_dev_prefix(tty), "console"); } int vtnr_from_tty(const char *tty) { @@ -660,8 +657,7 @@ int vtnr_from_tty(const char *tty) { assert(tty); - if (startswith(tty, "/dev/")) - tty += 5; + tty = skip_dev_prefix(tty); if (!startswith(tty, "tty") ) return -EINVAL; @@ -775,8 +771,7 @@ bool tty_is_vc_resolve(const char *tty) { assert(tty); - if (startswith(tty, "/dev/")) - tty += 5; + tty = skip_dev_prefix(tty); if (streq(tty, "console")) { tty = resolve_dev_console(&active); @@ -918,11 +913,9 @@ int getttyname_malloc(int fd, char **ret) { r = ttyname_r(fd, path, sizeof(path)); if (r == 0) { - const char *p; char *c; - p = startswith(path, "/dev/"); - c = strdup(p ?: path); + c = strdup(skip_dev_prefix(path)); if (!c) return -ENOMEM; diff --git a/src/core/execute.c b/src/core/execute.c index 55f9aab4b2..21488a0efc 100644 --- a/src/core/execute.c +++ b/src/core/execute.c @@ -3400,8 +3400,7 @@ static bool tty_may_match_dev_console(const char *tty) { if (!tty) return true; - if (startswith(tty, "/dev/")) - tty += 5; + tty = skip_dev_prefix(tty); /* trivial identity? */ if (streq(tty, "console")) diff --git a/src/test/test-path-util.c b/src/test/test-path-util.c index e5644246c2..e708d6ee50 100644 --- a/src/test/test-path-util.c +++ b/src/test/test-path-util.c @@ -588,6 +588,21 @@ static void test_systemd_installation_has_version(const char *path) { } } +static void test_skip_dev_prefix(void) { + + assert_se(streq(skip_dev_prefix("/"), "/")); + assert_se(streq(skip_dev_prefix("/dev"), "")); + assert_se(streq(skip_dev_prefix("/dev/"), "")); + assert_se(streq(skip_dev_prefix("/dev/foo"), "foo")); + assert_se(streq(skip_dev_prefix("/dev/foo/bar"), "foo/bar")); + assert_se(streq(skip_dev_prefix("//dev"), "")); + assert_se(streq(skip_dev_prefix("//dev//"), "")); + assert_se(streq(skip_dev_prefix("/dev///foo"), "foo")); + assert_se(streq(skip_dev_prefix("///dev///foo///bar"), "foo///bar")); + assert_se(streq(skip_dev_prefix("//foo"), "//foo")); + assert_se(streq(skip_dev_prefix("foo"), "foo")); +} + int main(int argc, char **argv) { log_set_max_level(LOG_DEBUG); log_parse_environment(); @@ -607,6 +622,7 @@ int main(int argc, char **argv) { test_file_in_same_dir(); test_filename_is_valid(); test_hidden_or_backup_file(); + test_skip_dev_prefix(); test_systemd_installation_has_version(argv[1]); /* NULL is OK */ From 27458ed62945be0f022ff69348a913793337108d Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 9 Aug 2017 19:03:39 +0200 Subject: [PATCH 2/3] tree-wide: use path_startswith() rather than startswith() where ever that's appropriate When checking path prefixes we really should use the right APIs, just in case people add multiple slashes to their paths... --- src/basic/terminal-util.c | 10 +++++++--- src/core/cgroup.c | 2 +- src/core/dbus-cgroup.c | 4 ++-- src/login/logind-utmp.c | 11 ++++++++--- src/sysctl/sysctl.c | 2 +- src/udev/udevadm-test-builtin.c | 3 ++- src/udev/udevadm-util.c | 5 +++-- 7 files changed, 24 insertions(+), 13 deletions(-) diff --git a/src/basic/terminal-util.c b/src/basic/terminal-util.c index c570c2e356..d665a181bc 100644 --- a/src/basic/terminal-util.c +++ b/src/basic/terminal-util.c @@ -55,6 +55,7 @@ #include "terminal-util.h" #include "time-util.h" #include "util.h" +#include "path-util.h" static volatile unsigned cached_columns = 0; static volatile unsigned cached_lines = 0; @@ -556,6 +557,7 @@ int terminal_vhangup(const char *name) { int vt_disallocate(const char *name) { _cleanup_close_ int fd = -1; + const char *e, *n; unsigned u; int r; @@ -563,7 +565,8 @@ int vt_disallocate(const char *name) { * (i.e. because it is the active one), at least clear it * entirely (including the scrollback buffer) */ - if (!startswith(name, "/dev/")) + e = path_startswith(name, "/dev/"); + if (!e) return -EINVAL; if (!tty_is_vc(name)) { @@ -582,10 +585,11 @@ int vt_disallocate(const char *name) { return 0; } - if (!startswith(name, "/dev/tty")) + n = startswith(e, "tty"); + if (!n) return -EINVAL; - r = safe_atou(name+8, &u); + r = safe_atou(n, &u); if (r < 0) return r; diff --git a/src/core/cgroup.c b/src/core/cgroup.c index 82d86a0ad4..94a4f9ae29 100644 --- a/src/core/cgroup.c +++ b/src/core/cgroup.c @@ -949,7 +949,7 @@ static void cgroup_context_apply(Unit *u, CGroupMask mask, ManagerState state) { acc[k++] = 0; - if (startswith(a->path, "/dev/")) + if (path_startswith(a->path, "/dev/")) whitelist_device(path, a->path, acc); else if ((val = startswith(a->path, "block-"))) whitelist_major(path, val, 'b', acc); diff --git a/src/core/dbus-cgroup.c b/src/core/dbus-cgroup.c index 4e6d1effaa..c1026e3f5b 100644 --- a/src/core/dbus-cgroup.c +++ b/src/core/dbus-cgroup.c @@ -1018,8 +1018,8 @@ int bus_cgroup_set_property( while ((r = sd_bus_message_read(message, "(ss)", &path, &rwm)) > 0) { - if ((!startswith(path, "/dev/") && - !startswith(path, "/run/systemd/inaccessible/") && + if ((!path_startswith(path, "/dev/") && + !path_startswith(path, "/run/systemd/inaccessible/") && !startswith(path, "block-") && !startswith(path, "char-")) || strpbrk(path, WHITESPACE)) diff --git a/src/login/logind-utmp.c b/src/login/logind-utmp.c index 311751c2db..00c4cbcf4f 100644 --- a/src/login/logind-utmp.c +++ b/src/login/logind-utmp.c @@ -31,6 +31,7 @@ #include "bus-util.h" #include "format-util.h" #include "logind.h" +#include "path-util.h" #include "special.h" #include "strv.h" #include "unit-name.h" @@ -60,15 +61,19 @@ _const_ static usec_t when_wall(usec_t n, usec_t elapse) { } bool logind_wall_tty_filter(const char *tty, void *userdata) { - Manager *m = userdata; + const char *p; assert(m); - if (!startswith(tty, "/dev/") || !m->scheduled_shutdown_tty) + if (!m->scheduled_shutdown_tty) return true; - return !streq(tty + 5, m->scheduled_shutdown_tty); + p = path_startswith(tty, "/dev/"); + if (!p) + return true; + + return !streq(p, m->scheduled_shutdown_tty); } static int warn_wall(Manager *m, usec_t n) { diff --git a/src/sysctl/sysctl.c b/src/sysctl/sysctl.c index b3587e249d..41a6dcbd1b 100644 --- a/src/sysctl/sysctl.c +++ b/src/sysctl/sysctl.c @@ -223,7 +223,7 @@ static int parse_argv(int argc, char *argv[]) { * sysctl name available. */ sysctl_normalize(optarg); - if (startswith(optarg, "/proc/sys")) + if (path_startswith(optarg, "/proc/sys")) p = strdup(optarg); else p = strappend("/proc/sys/", optarg); diff --git a/src/udev/udevadm-test-builtin.c b/src/udev/udevadm-test-builtin.c index 0b180d03eb..b5662be5c2 100644 --- a/src/udev/udevadm-test-builtin.c +++ b/src/udev/udevadm-test-builtin.c @@ -21,6 +21,7 @@ #include #include +#include "path-util.h" #include "string-util.h" #include "udev.h" @@ -80,7 +81,7 @@ static int adm_builtin(struct udev *udev, int argc, char *argv[]) { } /* add /sys if needed */ - if (!startswith(syspath, "/sys")) + if (!path_startswith(syspath, "/sys")) strscpyl(filename, sizeof(filename), "/sys", syspath, NULL); else strscpy(filename, sizeof(filename), syspath); diff --git a/src/udev/udevadm-util.c b/src/udev/udevadm-util.c index 3539c1d6ab..beda7c36bb 100644 --- a/src/udev/udevadm-util.c +++ b/src/udev/udevadm-util.c @@ -15,6 +15,7 @@ * along with this program. If not, see . */ +#include "path-util.h" #include "string-util.h" #include "udevadm-util.h" @@ -28,7 +29,7 @@ struct udev_device *find_device(struct udev *udev, if (prefix && !startswith(id, prefix)) id = strjoina(prefix, id); - if (startswith(id, "/dev/")) { + if (path_startswith(id, "/dev/")) { struct stat statbuf; char type; @@ -43,7 +44,7 @@ struct udev_device *find_device(struct udev *udev, return NULL; return udev_device_new_from_devnum(udev, type, statbuf.st_rdev); - } else if (startswith(id, "/sys/")) + } else if (path_startswith(id, "/sys/")) return udev_device_new_from_syspath(udev, id); else return NULL; From c9ed61e7a0149a5afff717b1450cdb821d39ab31 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 9 Aug 2017 19:04:36 +0200 Subject: [PATCH 3/3] pam_logind: skip leading /dev/ from PAM_TTY field before passing it on Apparently, PAM documents that the PAM_TTY should come with a /dev prefix, but we don't expect it so far, except that Wayland ends up setting it after all, the way the docs suggest. Hence, let's simply drop the /dev prefix if it is there. Fixes: #6516 --- src/login/pam_systemd.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/login/pam_systemd.c b/src/login/pam_systemd.c index df16907ed2..730e0f1b5a 100644 --- a/src/login/pam_systemd.c +++ b/src/login/pam_systemd.c @@ -46,6 +46,7 @@ #include "strv.h" #include "terminal-util.h" #include "util.h" +#include "path-util.h" static int parse_argv( pam_handle_t *handle, @@ -335,7 +336,9 @@ _public_ PAM_EXTERN int pam_sm_open_session( type ="tty"; class = "user"; tty = NULL; - } + } else + /* Chop off leading /dev prefix that some clients specify, but others do not. */ + tty = skip_dev_prefix(tty); /* If this fails vtnr will be 0, that's intended */ if (!isempty(cvtnr))