From 5a46d55fc8f3dc4a86f3994f96369d169c0b96bb Mon Sep 17 00:00:00 2001 From: Zbigniew Jędrzejewski-Szmek Date: Sun, 2 Oct 2016 15:51:27 +0200 Subject: path-util: add a function to peek into a container and guess systemd version This is a bit crude and only works for new systemd versions which have libsystemd-shared. --- src/basic/path-util.c | 68 +++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 68 insertions(+) (limited to 'src/basic/path-util.c') diff --git a/src/basic/path-util.c b/src/basic/path-util.c index b2fa81a294..c32e961af4 100644 --- a/src/basic/path-util.c +++ b/src/basic/path-util.c @@ -34,9 +34,11 @@ #include "alloc-util.h" #include "extract-word.h" #include "fs-util.h" +#include "glob-util.h" #include "log.h" #include "macro.h" #include "missing.h" +#include "parse-util.h" #include "path-util.h" #include "stat-util.h" #include "string-util.h" @@ -814,3 +816,69 @@ bool is_device_path(const char *path) { path_startswith(path, "/dev/") || path_startswith(path, "/sys/"); } + +int systemd_installation_has_version(const char *root, unsigned minimal_version) { + const char *pattern; + int r; + + /* Try to guess if systemd installation is later than the specified version. This + * is hacky and likely to yield false negatives, particularly if the installation + * is non-standard. False positives should be relatively rare. + */ + + NULSTR_FOREACH(pattern, + /* /lib works for systems without usr-merge, and for systems with a sane + * usr-merge, where /lib is a symlink to /usr/lib. /usr/lib is necessary + * for Gentoo which does a merge without making /lib a symlink. + */ + "lib/systemd/libsystemd-shared-*.so\0" + "usr/lib/systemd/libsystemd-shared-*.so\0") { + + _cleanup_strv_free_ char **names = NULL; + _cleanup_free_ char *path = NULL; + char *c, **name; + + path = prefix_root(root, pattern); + if (!path) + return -ENOMEM; + + r = glob_extend(&names, path); + if (r == -ENOENT) + continue; + if (r < 0) + return r; + + assert_se((c = endswith(path, "*.so"))); + *c = '\0'; /* truncate the glob part */ + + STRV_FOREACH(name, names) { + /* This is most likely to run only once, hence let's not optimize anything. */ + char *t, *t2; + unsigned version; + + t = startswith(*name, path); + if (!t) + continue; + + t2 = endswith(t, ".so"); + if (!t2) + continue; + + t2[0] = '\0'; /* truncate the suffix */ + + r = safe_atou(t, &version); + if (r < 0) { + log_debug_errno(r, "Found libsystemd shared at \"%s.so\", but failed to parse version: %m", *name); + continue; + } + + log_debug("Found libsystemd shared at \"%s.so\", version %u (%s).", + *name, version, + version >= minimal_version ? "OK" : "too old"); + if (version >= minimal_version) + return true; + } + } + + return false; +} -- cgit v1.2.3-54-g00ecf From 3ccb886283a1a98b549f44b6d33edeecc3768f1f Mon Sep 17 00:00:00 2001 From: Zbigniew Jędrzejewski-Szmek Date: Wed, 12 Oct 2016 05:12:11 -0400 Subject: Allow block and char classes in DeviceAllow bus properties (#4353) Allowed paths are unified betwen the configuration file parses and the bus property checker. The biggest change is that the bus code now allows "block-" and "char-" classes. In addition, path_startswith("/dev") was used in the bus code, and startswith("/dev") was used in the config file code. It seems reasonable to use path_startswith() which allows a slightly broader class of strings. Fixes #3935. --- src/basic/path-util.c | 11 ++++++++--- src/basic/path-util.h | 1 + src/core/load-fragment.c | 4 +--- src/shared/bus-unit-util.c | 2 +- 4 files changed, 11 insertions(+), 7 deletions(-) (limited to 'src/basic/path-util.c') diff --git a/src/basic/path-util.c b/src/basic/path-util.c index c32e961af4..a76963aa9f 100644 --- a/src/basic/path-util.c +++ b/src/basic/path-util.c @@ -812,9 +812,14 @@ bool is_device_path(const char *path) { /* Returns true on paths that refer to a device, either in * sysfs or in /dev */ - return - path_startswith(path, "/dev/") || - path_startswith(path, "/sys/"); + return path_startswith(path, "/dev/") || + path_startswith(path, "/sys/"); +} + +bool is_deviceallow_pattern(const char *path) { + return path_startswith(path, "/dev/") || + startswith(path, "block-") || + startswith(path, "char-"); } int systemd_installation_has_version(const char *root, unsigned minimal_version) { diff --git a/src/basic/path-util.h b/src/basic/path-util.h index 78472f0961..66545f52d9 100644 --- a/src/basic/path-util.h +++ b/src/basic/path-util.h @@ -125,5 +125,6 @@ char *file_in_same_dir(const char *path, const char *filename); bool hidden_or_backup_file(const char *filename) _pure_; bool is_device_path(const char *path); +bool is_deviceallow_pattern(const char *path); int systemd_installation_has_version(const char *root, unsigned minimal_version); diff --git a/src/core/load-fragment.c b/src/core/load-fragment.c index 8f067b5586..06c156a623 100644 --- a/src/core/load-fragment.c +++ b/src/core/load-fragment.c @@ -3084,9 +3084,7 @@ int config_parse_device_allow( if (!path) return log_oom(); - if (!startswith(path, "/dev/") && - !startswith(path, "block-") && - !startswith(path, "char-")) { + if (!is_deviceallow_pattern(path)) { log_syntax(unit, LOG_ERR, filename, line, 0, "Invalid device node path '%s'. Ignoring.", path); return 0; } diff --git a/src/shared/bus-unit-util.c b/src/shared/bus-unit-util.c index c6bd2f145c..a550a370b5 100644 --- a/src/shared/bus-unit-util.c +++ b/src/shared/bus-unit-util.c @@ -303,7 +303,7 @@ int bus_append_unit_property_assignment(sd_bus_message *m, const char *assignmen rwm = ""; } - if (!path_startswith(path, "/dev")) { + if (!is_deviceallow_pattern(path)) { log_error("%s is not a device file in /dev.", path); return -EINVAL; } -- cgit v1.2.3-54-g00ecf From 3b319885c4febb5f7ea9b5ab31c3395548ed6886 Mon Sep 17 00:00:00 2001 From: Zbigniew Jędrzejewski-Szmek Date: Sun, 16 Oct 2016 19:23:35 -0400 Subject: tree-wide: introduce free_and_replace helper It's a common pattern, so add a helper for it. A macro is necessary because a function that takes a pointer to a pointer would be type specific, similarly to cleanup functions. Seems better to use a macro. --- coccinelle/free_and_replace.cocci | 15 +++++++++++++++ src/basic/alloc-util.h | 8 ++++++++ src/basic/fs-util.c | 8 ++------ src/basic/path-util.c | 4 +--- src/core/load-fragment.c | 10 ++-------- src/core/mount.c | 14 +++----------- src/core/service.c | 4 +--- src/journal-remote/journal-upload.c | 4 +--- src/shared/install.c | 10 ++-------- 9 files changed, 35 insertions(+), 42 deletions(-) create mode 100644 coccinelle/free_and_replace.cocci (limited to 'src/basic/path-util.c') diff --git a/coccinelle/free_and_replace.cocci b/coccinelle/free_and_replace.cocci new file mode 100644 index 0000000000..9dcdbf4d42 --- /dev/null +++ b/coccinelle/free_and_replace.cocci @@ -0,0 +1,15 @@ +@@ +expression p, q; +@@ +- free(p); +- p = q; +- q = NULL; +- return 0; ++ return free_and_replace(p, q); +@@ +expression p, q; +@@ +- free(p); +- p = q; +- q = NULL; ++ free_and_replace(p, q); diff --git a/src/basic/alloc-util.h b/src/basic/alloc-util.h index ceeee519b7..a44dd473c1 100644 --- a/src/basic/alloc-util.h +++ b/src/basic/alloc-util.h @@ -43,6 +43,14 @@ static inline void *mfree(void *memory) { return NULL; } +#define free_and_replace(a, b) \ + ({ \ + free(a); \ + (a) = (b); \ + (b) = NULL; \ + 0; \ + }) + void* memdup(const void *p, size_t l) _alloc_(2); static inline void freep(void *p) { diff --git a/src/basic/fs-util.c b/src/basic/fs-util.c index 86d9ad7e36..48952a1c26 100644 --- a/src/basic/fs-util.c +++ b/src/basic/fs-util.c @@ -678,9 +678,7 @@ int chase_symlinks(const char *path, const char *_root, char **ret) { !path_startswith(parent, root)) return -EINVAL; - free(done); - done = parent; - parent = NULL; + free_and_replace(done, parent); fd_parent = openat(fd, "..", O_CLOEXEC|O_NOFOLLOW|O_PATH); if (fd_parent < 0) @@ -724,9 +722,7 @@ int chase_symlinks(const char *path, const char *_root, char **ret) { if (fd < 0) return -errno; - free(buffer); - buffer = destination; - destination = NULL; + free_and_replace(buffer, destination); todo = buffer; free(done); diff --git a/src/basic/path-util.c b/src/basic/path-util.c index a76963aa9f..0f5b20cf05 100644 --- a/src/basic/path-util.c +++ b/src/basic/path-util.c @@ -288,9 +288,7 @@ char **path_strv_resolve(char **l, const char *prefix) { } else { /* canonicalized path goes outside of * prefix, keep the original path instead */ - free(u); - u = orig; - orig = NULL; + free_and_replace(u, orig); } } else free(t); diff --git a/src/core/load-fragment.c b/src/core/load-fragment.c index 06c156a623..8cc7a8e765 100644 --- a/src/core/load-fragment.c +++ b/src/core/load-fragment.c @@ -1591,11 +1591,7 @@ int config_parse_fdname( return 0; } - free(s->fdname); - s->fdname = p; - p = NULL; - - return 0; + return free_and_replace(s->fdname, p); } int config_parse_service_sockets( @@ -2052,9 +2048,7 @@ int config_parse_working_directory( return 0; } - free(c->working_directory); - c->working_directory = k; - k = NULL; + free_and_replace(c->working_directory, k); c->working_directory_home = false; } diff --git a/src/core/mount.c b/src/core/mount.c index 15619dffe3..da480001e1 100644 --- a/src/core/mount.c +++ b/src/core/mount.c @@ -1484,17 +1484,9 @@ static int mount_setup_unit( MOUNT(u)->from_proc_self_mountinfo = true; - free(p->what); - p->what = w; - w = NULL; - - free(p->options); - p->options = o; - o = NULL; - - free(p->fstype); - p->fstype = f; - f = NULL; + free_and_replace(p->what, w); + free_and_replace(p->options, o); + free_and_replace(p->fstype, f); if (load_extras) { r = mount_add_extras(MOUNT(u)); diff --git a/src/core/service.c b/src/core/service.c index 63045ede55..c949de9cbe 100644 --- a/src/core/service.c +++ b/src/core/service.c @@ -3088,9 +3088,7 @@ static void service_notify_message(Unit *u, pid_t pid, char **tags, FDSet *fds) if (!streq_ptr(s->status_text, t)) { - free(s->status_text); - s->status_text = t; - t = NULL; + free_and_replace(s->status_text, t); notify_dbus = true; } diff --git a/src/journal-remote/journal-upload.c b/src/journal-remote/journal-upload.c index c0f967ab94..61190ff83c 100644 --- a/src/journal-remote/journal-upload.c +++ b/src/journal-remote/journal-upload.c @@ -527,9 +527,7 @@ static int perform_upload(Uploader *u) { log_debug("Upload finished successfully with code %ld: %s", status, strna(u->answer)); - free(u->last_cursor); - u->last_cursor = u->current_cursor; - u->current_cursor = NULL; + free_and_replace(u->last_cursor, u->current_cursor); return update_cursor_state(u); } diff --git a/src/shared/install.c b/src/shared/install.c index c1ef62b337..f70b3e3dd5 100644 --- a/src/shared/install.c +++ b/src/shared/install.c @@ -1107,11 +1107,7 @@ static int config_parse_default_instance( if (!unit_instance_is_valid(printed)) return -EINVAL; - free(i->default_instance); - i->default_instance = printed; - printed = NULL; - - return 0; + return free_and_replace(i->default_instance, printed); } static int unit_file_load( @@ -1357,9 +1353,7 @@ static int install_info_follow( if (!streq(basename(i->symlink_target), i->name)) return -EXDEV; - free(i->path); - i->path = i->symlink_target; - i->symlink_target = NULL; + free_and_replace(i->path, i->symlink_target); i->type = _UNIT_FILE_TYPE_INVALID; return unit_file_load_or_readlink(c, i, i->path, root_dir, flags); -- cgit v1.2.3-54-g00ecf From 0470289b6e93be7a65b1b3b2a2cc829bd20e08c8 Mon Sep 17 00:00:00 2001 From: Zbigniew Jędrzejewski-Szmek Date: Sun, 30 Oct 2016 10:21:29 -0400 Subject: tests: clarify test_path_startswith return value (#4508) A pendant for #4481. --- src/basic/path-util.c | 10 ++++++++++ src/test/test-path-util.c | 41 +++++++++++++++++++++++++++++++---------- 2 files changed, 41 insertions(+), 10 deletions(-) (limited to 'src/basic/path-util.c') diff --git a/src/basic/path-util.c b/src/basic/path-util.c index 0f5b20cf05..fd38f51c4c 100644 --- a/src/basic/path-util.c +++ b/src/basic/path-util.c @@ -354,6 +354,16 @@ char* path_startswith(const char *path, const char *prefix) { assert(path); assert(prefix); + /* Returns a pointer to the start of the first component after the parts matched by + * the prefix, iff + * - both paths are absolute or both paths are relative, + * and + * - each component in prefix in turn matches a component in path at the same position. + * An empty string will be returned when the prefix and path are equivalent. + * + * Returns NULL otherwise. + */ + if ((path[0] == '/') != (prefix[0] == '/')) return NULL; diff --git a/src/test/test-path-util.c b/src/test/test-path-util.c index 0b10d8e25e..a6a09a0031 100644 --- a/src/test/test-path-util.c +++ b/src/test/test-path-util.c @@ -263,16 +263,37 @@ static void test_strv_resolve(void) { } static void test_path_startswith(void) { - assert_se(path_startswith("/foo/bar/barfoo/", "/foo")); - assert_se(path_startswith("/foo/bar/barfoo/", "/foo/")); - assert_se(path_startswith("/foo/bar/barfoo/", "/")); - assert_se(path_startswith("/foo/bar/barfoo/", "////")); - assert_se(path_startswith("/foo/bar/barfoo/", "/foo//bar/////barfoo///")); - assert_se(path_startswith("/foo/bar/barfoo/", "/foo/bar/barfoo////")); - assert_se(path_startswith("/foo/bar/barfoo/", "/foo/bar///barfoo/")); - assert_se(path_startswith("/foo/bar/barfoo/", "/foo////bar/barfoo/")); - assert_se(path_startswith("/foo/bar/barfoo/", "////foo/bar/barfoo/")); - assert_se(path_startswith("/foo/bar/barfoo/", "/foo/bar/barfoo")); + const char *p; + + p = path_startswith("/foo/bar/barfoo/", "/foo"); + assert_se(streq_ptr(p, "bar/barfoo/")); + + p = path_startswith("/foo/bar/barfoo/", "/foo/"); + assert_se(streq_ptr(p, "bar/barfoo/")); + + p = path_startswith("/foo/bar/barfoo/", "/"); + assert_se(streq_ptr(p, "foo/bar/barfoo/")); + + p = path_startswith("/foo/bar/barfoo/", "////"); + assert_se(streq_ptr(p, "foo/bar/barfoo/")); + + p = path_startswith("/foo/bar/barfoo/", "/foo//bar/////barfoo///"); + assert_se(streq_ptr(p, "")); + + p = path_startswith("/foo/bar/barfoo/", "/foo/bar/barfoo////"); + assert_se(streq_ptr(p, "")); + + p = path_startswith("/foo/bar/barfoo/", "/foo/bar///barfoo/"); + assert_se(streq_ptr(p, "")); + + p = path_startswith("/foo/bar/barfoo/", "/foo////bar/barfoo/"); + assert_se(streq_ptr(p, "")); + + p = path_startswith("/foo/bar/barfoo/", "////foo/bar/barfoo/"); + assert_se(streq_ptr(p, "")); + + p = path_startswith("/foo/bar/barfoo/", "/foo/bar/barfoo"); + assert_se(streq_ptr(p, "")); assert_se(!path_startswith("/foo/bar/barfoo/", "/foo/bar/barfooa/")); assert_se(!path_startswith("/foo/bar/barfoo/", "/foo/bar/barfooa")); -- cgit v1.2.3-54-g00ecf