From 9b674e25817cdda29fdb393f9fcadc34517e179e Mon Sep 17 00:00:00 2001 From: Zbigniew Jędrzejewski-Szmek Date: Thu, 6 Dec 2018 11:27:40 +0100 Subject: core/device: fix typo --- src/core/device.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/device.c b/src/core/device.c index 5acd9b7a7..f8b6961d3 100644 --- a/src/core/device.c +++ b/src/core/device.c @@ -821,7 +821,7 @@ static void device_enumerate(Manager *m) { r = sd_device_enumerator_new(&e); if (r < 0) { - log_error_errno(r, "Failed to alloacte device enumerator: %m"); + log_error_errno(r, "Failed to allocate device enumerator: %m"); goto fail; } -- cgit v1.2.3-65-gdbad From 296acffe45a74a8c9cca393494760169b5a4afd3 Mon Sep 17 00:00:00 2001 From: Zbigniew Jędrzejewski-Szmek Date: Fri, 7 Dec 2018 16:12:19 +0100 Subject: When parsing paths, reject anything above PATH_MAX The check for length is done after path_simplify(), to be nice to paths which are constructed using specifiers, and have duplicate slashes and stuff. --- src/basic/path-util.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/basic/path-util.c b/src/basic/path-util.c index 995f39f16..221517303 100644 --- a/src/basic/path-util.c +++ b/src/basic/path-util.c @@ -1139,5 +1139,12 @@ int path_simplify_and_warn( return -EINVAL; } + if (!path_is_valid(path)) { + log_syntax(unit, LOG_ERR, filename, line, 0, + "%s= path has invalid length (%zu bytes)%s.", + lvalue, strlen(path), fatal ? "" : ", ignoring"); + return -EINVAL; + } + return 0; } -- cgit v1.2.3-65-gdbad From f8703ed7e51121cd42bfb05fe9fe2a626118b07b Mon Sep 17 00:00:00 2001 From: Zbigniew Jędrzejewski-Szmek Date: Fri, 7 Dec 2018 16:16:39 +0100 Subject: basic/path-util: line-break PATH_FOREACH_PREFIX macros Now I can see what they do :] --- src/basic/path-util.h | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/src/basic/path-util.h b/src/basic/path-util.h index 78db41b85..094aa47c0 100644 --- a/src/basic/path-util.h +++ b/src/basic/path-util.h @@ -97,12 +97,24 @@ int mkfs_exists(const char *fstype); /* Iterates through the path prefixes of the specified path, going up * the tree, to root. Also returns "" (and not "/"!) for the root * directory. Excludes the specified directory itself */ -#define PATH_FOREACH_PREFIX(prefix, path) \ - for (char *_slash = ({ path_simplify(strcpy(prefix, path), false); streq(prefix, "/") ? NULL : strrchr(prefix, '/'); }); _slash && ((*_slash = 0), true); _slash = strrchr((prefix), '/')) +#define PATH_FOREACH_PREFIX(prefix, path) \ + for (char *_slash = ({ \ + path_simplify(strcpy(prefix, path), false); \ + streq(prefix, "/") ? NULL : strrchr(prefix, '/'); \ + }); \ + _slash && ((*_slash = 0), true); \ + _slash = strrchr((prefix), '/')) /* Same as PATH_FOREACH_PREFIX but also includes the specified path itself */ -#define PATH_FOREACH_PREFIX_MORE(prefix, path) \ - for (char *_slash = ({ path_simplify(strcpy(prefix, path), false); if (streq(prefix, "/")) prefix[0] = 0; strrchr(prefix, 0); }); _slash && ((*_slash = 0), true); _slash = strrchr((prefix), '/')) +#define PATH_FOREACH_PREFIX_MORE(prefix, path) \ + for (char *_slash = ({ \ + path_simplify(strcpy(prefix, path), false); \ + if (streq(prefix, "/")) \ + prefix[0] = 0; \ + strrchr(prefix, 0); \ + }); \ + _slash && ((*_slash = 0), true); \ + _slash = strrchr((prefix), '/')) char *prefix_root(const char *root, const char *path); -- cgit v1.2.3-65-gdbad From 60473f0c23674bbde9f1b3cf1a2222a89c254886 Mon Sep 17 00:00:00 2001 From: Zbigniew Jędrzejewski-Szmek Date: Fri, 7 Dec 2018 16:22:10 +0100 Subject: pid1: fix (harmless) off-by-one in PATH_MAX comparison PATH_MAX is supposed to include the terminating NUL byte. But we already check that there is no NUL byte in the specified path. Hence the maximum length we can expect is PATH_MAX - 1. This doesn't change much, but makes this use of PATH_MAX consistent with the rest of the codebase. --- src/core/manager.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/manager.c b/src/core/manager.c index 871047e62..2398dcf4e 100644 --- a/src/core/manager.c +++ b/src/core/manager.c @@ -2222,7 +2222,7 @@ static unsigned manager_dispatch_dbus_queue(Manager *m) { 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]; + char buf[PATH_MAX]; ssize_t n; n = recv(fd, buf, sizeof(buf), 0); -- cgit v1.2.3-65-gdbad From 4cb06c5949eba752f87dd6ad74620cc0e5cabf4a Mon Sep 17 00:00:00 2001 From: Zbigniew Jędrzejewski-Szmek Date: Fri, 7 Dec 2018 16:38:03 +0100 Subject: Use VLA instead of alloca The test is the same, but an array is more readable. --- src/core/unit.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/core/unit.c b/src/core/unit.c index e1b6e9f11..a9303a0f3 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -4580,7 +4580,6 @@ int unit_kill_context( int unit_require_mounts_for(Unit *u, const char *path, UnitDependencyMask mask) { _cleanup_free_ char *p = NULL; - char *prefix; UnitDependencyInfo di; int r; @@ -4620,7 +4619,7 @@ int unit_require_mounts_for(Unit *u, const char *path, UnitDependencyMask mask) return r; p = NULL; - prefix = alloca(strlen(path) + 1); + char prefix[strlen(path) + 1]; PATH_FOREACH_PREFIX_MORE(prefix, path) { Set *x; -- cgit v1.2.3-65-gdbad From a5dfc36ce60bf57ea319756a07a4a8a9f2a58b69 Mon Sep 17 00:00:00 2001 From: Zbigniew Jędrzejewski-Szmek Date: Fri, 7 Dec 2018 16:49:20 +0100 Subject: fuzz-unit-file: add one more test case There seems to be no error per se. RequiresMountsFor=%s%s%s..%s%s%s is expanded to RequiresMountsFor=/bin/zsh/bin/zsh/bin/zsh/bin/zsh/..., which takes a bit of time, and then we iterate over this a few times, creating a hashmap with a hashmap for each prefix of the path, each with one item pointing back to the original unit. Takes about 0.8 s on my machine. --- test/fuzz/fuzz-unit-file/oss-fuzz-11569 | Bin 0 -> 277466 bytes 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 test/fuzz/fuzz-unit-file/oss-fuzz-11569 diff --git a/test/fuzz/fuzz-unit-file/oss-fuzz-11569 b/test/fuzz/fuzz-unit-file/oss-fuzz-11569 new file mode 100644 index 000000000..c49b8c6b1 Binary files /dev/null and b/test/fuzz/fuzz-unit-file/oss-fuzz-11569 differ -- cgit v1.2.3-65-gdbad