From e911de996a72af7659e4019f03b80f11c476f3f3 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 21 Apr 2015 20:22:51 +0200 Subject: core: make unit deserialization more defensive --- src/core/unit.c | 97 +++++++++++++++++++++++++++++++------------------- src/shared/time-util.c | 15 ++++---- src/shared/time-util.h | 2 +- 3 files changed, 71 insertions(+), 43 deletions(-) (limited to 'src') diff --git a/src/core/unit.c b/src/core/unit.c index 494dee4156..e921b48fc4 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -2644,6 +2644,40 @@ void unit_serialize_item(Unit *u, FILE *f, const char *key, const char *value) { fprintf(f, "%s=%s\n", key, value); } +static int unit_set_cgroup_path(Unit *u, const char *path) { + _cleanup_free_ char *p = NULL; + int r; + + assert(u); + + if (path) { + p = strdup(path); + if (!p) + return -ENOMEM; + } else + p = NULL; + + if (streq_ptr(u->cgroup_path, p)) + return 0; + + if (p) { + r = hashmap_put(u->manager->cgroup_unit, p, u); + if (r < 0) + return r; + } + + if (u->cgroup_path) { + log_unit_debug(u->id, "%s: Changing cgroup path from %s to %s.", u->id, u->cgroup_path, strna(p)); + hashmap_remove(u->manager->cgroup_unit, u->cgroup_path); + free(u->cgroup_path); + } + + u->cgroup_path = p; + p = NULL; + + return 0; +} + int unit_deserialize(Unit *u, FILE *f, FDSet *fds) { ExecRuntime **rt = NULL; size_t offset; @@ -2671,7 +2705,7 @@ int unit_deserialize(Unit *u, FILE *f, FDSet *fds) { l = strstrip(line); /* End marker */ - if (l[0] == 0) + if (isempty(l)) return 0; k = strcspn(l, "="); @@ -2689,7 +2723,7 @@ int unit_deserialize(Unit *u, FILE *f, FDSet *fds) { j = job_new_raw(u); if (!j) - return -ENOMEM; + return log_oom(); r = job_deserialize(j, f, fds); if (r < 0) { @@ -2739,61 +2773,48 @@ int unit_deserialize(Unit *u, FILE *f, FDSet *fds) { dual_timestamp_deserialize(v, &u->assert_timestamp); continue; } else if (streq(l, "condition-result")) { - int b; - b = parse_boolean(v); - if (b < 0) - log_debug("Failed to parse condition result value %s", v); + r = parse_boolean(v); + if (r < 0) + log_debug("Failed to parse condition result value %s, ignoring.", v); else - u->condition_result = b; + u->condition_result = r; continue; } else if (streq(l, "assert-result")) { - int b; - b = parse_boolean(v); - if (b < 0) - log_debug("Failed to parse assert result value %s", v); + r = parse_boolean(v); + if (r < 0) + log_debug("Failed to parse assert result value %s, ignoring.", v); else - u->assert_result = b; + u->assert_result = r; continue; } else if (streq(l, "transient")) { - int b; - b = parse_boolean(v); - if (b < 0) - log_debug("Failed to parse transient bool %s", v); + r = parse_boolean(v); + if (r < 0) + log_debug("Failed to parse transient bool %s, ignoring.", v); else - u->transient = b; + u->transient = r; continue; + } else if (streq(l, "cpuacct-usage-base")) { r = safe_atou64(v, &u->cpuacct_usage_base); if (r < 0) - log_debug("Failed to parse CPU usage %s", v); + log_debug("Failed to parse CPU usage %s, ignoring.", v); continue; - } else if (streq(l, "cgroup")) { - char *s; - - s = strdup(v); - if (!s) - return -ENOMEM; - - if (u->cgroup_path) { - void *p; - p = hashmap_remove(u->manager->cgroup_unit, u->cgroup_path); - log_info("Removing cgroup_path %s from hashmap (%p)", u->cgroup_path, p); - free(u->cgroup_path); - } + } else if (streq(l, "cgroup")) { - u->cgroup_path = s; - assert(hashmap_put(u->manager->cgroup_unit, s, u) == 1); + r = unit_set_cgroup_path(u, v); + if (r < 0) + log_debug_errno(r, "Failed to set cgroup path %s, ignoring: %m", v); continue; } @@ -2801,15 +2822,19 @@ int unit_deserialize(Unit *u, FILE *f, FDSet *fds) { if (unit_can_serialize(u)) { if (rt) { r = exec_runtime_deserialize_item(rt, u, l, v, fds); - if (r < 0) - return r; + if (r < 0) { + log_unit_warning(u->id, "Failed to deserialize runtime parameter '%s', ignoring.", l); + continue; + } + + /* Returns positive if key was handled by the call */ if (r > 0) continue; } r = UNIT_VTABLE(u)->deserialize_item(u, l, v, fds); if (r < 0) - return r; + log_unit_warning(u->id, "Failed to deserialize unit parameter '%s', ignoring.", l); } } } diff --git a/src/shared/time-util.c b/src/shared/time-util.c index 1c36c577c4..12f1b193be 100644 --- a/src/shared/time-util.c +++ b/src/shared/time-util.c @@ -398,18 +398,21 @@ void dual_timestamp_serialize(FILE *f, const char *name, dual_timestamp *t) { t->monotonic); } -void dual_timestamp_deserialize(const char *value, dual_timestamp *t) { +int dual_timestamp_deserialize(const char *value, dual_timestamp *t) { unsigned long long a, b; assert(value); assert(t); - if (sscanf(value, "%llu %llu", &a, &b) != 2) - log_debug("Failed to parse finish timestamp value %s", value); - else { - t->realtime = a; - t->monotonic = b; + if (sscanf(value, "%llu %llu", &a, &b) != 2) { + log_debug("Failed to parse finish timestamp value %s.", value); + return -EINVAL; } + + t->realtime = a; + t->monotonic = b; + + return 0; } int parse_timestamp(const char *t, usec_t *usec) { diff --git a/src/shared/time-util.h b/src/shared/time-util.h index fca8a4db9b..7a64d454a0 100644 --- a/src/shared/time-util.h +++ b/src/shared/time-util.h @@ -94,7 +94,7 @@ char *format_timestamp_relative(char *buf, size_t l, usec_t t); char *format_timespan(char *buf, size_t l, usec_t t, usec_t accuracy); void dual_timestamp_serialize(FILE *f, const char *name, dual_timestamp *t); -void dual_timestamp_deserialize(const char *value, dual_timestamp *t); +int dual_timestamp_deserialize(const char *value, dual_timestamp *t); int parse_timestamp(const char *t, usec_t *usec); -- cgit v1.2.3-54-g00ecf