Merge pull request #8461 from keszybz/oss-fuzz-fixes

Oss fuzz fixes

src/basic/calendarspec.c

@@ -43,6 +43,12 @@
 #define MIN_YEAR 1970
 #define MAX_YEAR 2199
 
+/* An arbitrary limit on the length of the chains of components. We don't want to
+ * build a very long linked list, which would be slow to iterate over and might cause
+ * our stack to overflow. It's unlikely that legitimate uses require more than a few
+ * linked compenents anyway. */
+#define CALENDARSPEC_COMPONENTS_MAX 240
+
 static void free_chain(CalendarComponent *c) {
         CalendarComponent *n;
 
@@ -618,15 +624,16 @@ static int calendarspec_from_time_t(CalendarSpec *c, time_t time) {
         return 0;
 }
 
-static int prepend_component(const char **p, bool usec, CalendarComponent **c) {
+static int prepend_component(const char **p, bool usec, unsigned nesting, CalendarComponent **c) {
         int r, start, stop = -1, repeat = 0;
         CalendarComponent *cc;
-        const char *e;
+        const char *e = *p;
 
         assert(p);
         assert(c);
 
-        e = *p;
+        if (nesting > CALENDARSPEC_COMPONENTS_MAX)
+                return -ENOBUFS;
 
         r = parse_component_decimal(&e, usec, &start);
         if (r < 0)
@@ -668,7 +675,7 @@ static int prepend_component(const char **p, bool usec, CalendarComponent **c) {
 
         if (*e ==',') {
                 *p += 1;
-                return prepend_component(p, usec, c);
+                return prepend_component(p, usec, nesting + 1, c);
         }
 
         return 0;
@@ -697,7 +704,7 @@ static int parse_chain(const char **p, bool usec, CalendarComponent **c) {
                 return 0;
         }
 
-        r = prepend_component(&t, usec, &cc);
+        r = prepend_component(&t, usec, 0, &cc);
         if (r < 0) {
                 free_chain(cc);
                 return r;

src/basic/cgroup-util.c

@@ -1977,6 +1977,14 @@ int cg_slice_to_path(const char *unit, char **ret) {
                 _cleanup_free_ char *escaped = NULL;
                 char n[dash - p + sizeof(".slice")];
 
+#ifdef FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION
+                /* msan doesn't instrument stpncpy, so it thinks
+                 * n is later used unitialized:
+                 * https://github.com/google/sanitizers/issues/926
+                 */
+                zero(n);
+#endif
+
                 /* Don't allow trailing or double dashes */
                 if (IN_SET(dash[1], 0, '-'))
                         return -EINVAL;
@@ -2475,7 +2483,7 @@ static int cg_unified_update(void) {
                 return 0;
 
         if (statfs("/sys/fs/cgroup/", &fs) < 0)
-                return log_debug_errno(errno, "statfs(\"/sys/fs/cgroup/\" failed: %m");
+                return log_debug_errno(errno, "statfs(\"/sys/fs/cgroup/\") failed: %m");
 
         if (F_TYPE_EQUAL(fs.f_type, CGROUP2_SUPER_MAGIC)) {
                 log_debug("Found cgroup2 on /sys/fs/cgroup/, full unified hierarchy");

src/basic/ioprio.h

@@ -10,12 +10,13 @@
 /*
  * Gives us 8 prio classes with 13-bits of data for each class
  */
-#define IOPRIO_BITS             (16)
-#define IOPRIO_CLASS_SHIFT      (13)
-#define IOPRIO_PRIO_MASK        ((1UL << IOPRIO_CLASS_SHIFT) - 1)
+#define IOPRIO_BITS             16
+#define IOPRIO_N_CLASSES        8
+#define IOPRIO_CLASS_SHIFT      13
+#define IOPRIO_PRIO_DATA_MASK   ((1UL << IOPRIO_CLASS_SHIFT) - 1)
 
 #define IOPRIO_PRIO_CLASS(mask) ((mask) >> IOPRIO_CLASS_SHIFT)
-#define IOPRIO_PRIO_DATA(mask)  ((mask) & IOPRIO_PRIO_MASK)
+#define IOPRIO_PRIO_DATA(mask)  ((mask) & IOPRIO_PRIO_DATA_MASK)
 #define IOPRIO_PRIO_VALUE(class, data)  (((class) << IOPRIO_CLASS_SHIFT) | data)
 
 #define ioprio_valid(mask)      (IOPRIO_PRIO_CLASS((mask)) != IOPRIO_CLASS_NONE)

src/basic/process-util.c

@@ -1466,7 +1466,7 @@ static const char *const ioprio_class_table[] = {
         [IOPRIO_CLASS_IDLE] = "idle"
 };
 
-DEFINE_STRING_TABLE_LOOKUP_WITH_FALLBACK(ioprio_class, int, INT_MAX);
+DEFINE_STRING_TABLE_LOOKUP_WITH_FALLBACK(ioprio_class, int, IOPRIO_N_CLASSES);
 
 static const char *const sigchld_code_table[] = {
         [CLD_EXITED] = "exited",

src/core/service.c

@@ -396,6 +396,9 @@ static void service_done(Unit *u) {
 
         s->bus_name_owner = mfree(s->bus_name_owner);
 
+        s->usb_function_descriptors = mfree(s->usb_function_descriptors);
+        s->usb_function_strings = mfree(s->usb_function_strings);
+
         service_close_socket_fd(s);
         s->peer = socket_peer_unref(s->peer);
 

src/core/unit.c

@@ -4564,7 +4564,8 @@ int unit_kill_context(
 }
 
 int unit_require_mounts_for(Unit *u, const char *path, UnitDependencyMask mask) {
-        char prefix[strlen(path) + 1], *p;
+        _cleanup_free_ char *p = NULL;
+        char *prefix;
         UnitDependencyInfo di;
         int r;
 
@@ -4587,34 +4588,30 @@ int unit_require_mounts_for(Unit *u, const char *path, UnitDependencyMask mask)
         if (!p)
                 return -ENOMEM;
 
-        path_kill_slashes(p);
+        path = path_kill_slashes(p);
 
-        if (!path_is_normalized(p)) {
-                free(p);
+        if (!path_is_normalized(path))
                 return -EPERM;
-        }
 
-        if (hashmap_contains(u->requires_mounts_for, p)) {
-                free(p);
+        if (hashmap_contains(u->requires_mounts_for, path))
                 return 0;
-        }
 
         di = (UnitDependencyInfo) {
                 .origin_mask = mask
         };
 
-        r = hashmap_put(u->requires_mounts_for, p, di.data);
-        if (r < 0) {
-                free(p);
+        r = hashmap_put(u->requires_mounts_for, path, di.data);
+        if (r < 0)
                 return r;
-        }
+        p = NULL;
 
-        PATH_FOREACH_PREFIX_MORE(prefix, p) {
+        prefix = alloca(strlen(path) + 1);
+        PATH_FOREACH_PREFIX_MORE(prefix, path) {
                 Set *x;
 
                 x = hashmap_get(u->manager->units_requiring_mounts_for, prefix);
                 if (!x) {
-                        char *q;
+                        _cleanup_free_ char *q = NULL;
 
                         r = hashmap_ensure_allocated(&u->manager->units_requiring_mounts_for, &path_hash_ops);
                         if (r < 0)
@@ -4625,17 +4622,15 @@ int unit_require_mounts_for(Unit *u, const char *path, UnitDependencyMask mask)
                                 return -ENOMEM;
 
                         x = set_new(NULL);
-                        if (!x) {
-                                free(q);
+                        if (!x)
                                 return -ENOMEM;
-                        }
 
                         r = hashmap_put(u->manager->units_requiring_mounts_for, q, x);
                         if (r < 0) {
-                                free(q);
                                 set_free(x);
                                 return r;
                         }
+                        q = NULL;
                 }
 
                 r = set_put(x, u);

src/fuzz/fuzz-unit-file.c

@@ -18,6 +18,7 @@ int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) {
         _cleanup_(manager_freep) Manager *m = NULL;
         Unit *u;
         const char *name;
+        long offset;
 
         if (size == 0)
                 return 0;
@@ -35,6 +36,23 @@ int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) {
         if (!unit_vtable[t]->load)
                 return 0;
 
+        offset = ftell(f);
+        assert_se(offset >= 0);
+
+        for (;;) {
+                _cleanup_free_ char *l = NULL;
+
+                if (read_line(f, LINE_MAX, &l) <= 0)
+                        break;
+
+                if (startswith(l, "ListenNetlink="))
+                        /* ListenNetlink causes a false positive in msan,
+                         * let's skip this for now. */
+                        return 0;
+        }
+
+        assert_se(fseek(f, offset, SEEK_SET) == 0);
+
         /* We don't want to fill the logs with messages about parse errors.
          * Disable most logging if not running standalone */
         if (!getenv("SYSTEMD_LOG_LEVEL"))

src/test/test-calendarspec.c

@@ -259,6 +259,7 @@ int main(int argc, char* argv[]) {
         assert_se(calendar_spec_from_string("00:00/60", &c) < 0);
         assert_se(calendar_spec_from_string("00:00:2300", &c) < 0);
         assert_se(calendar_spec_from_string("00:00:18446744073709551615", &c) < 0);
+        assert_se(calendar_spec_from_string("@88588582097858858", &c) == -ERANGE);
 
         test_timestamp();
         test_hourly_bug_4031();

src/test/test-process-util.c

@@ -541,8 +541,33 @@ static void test_pid_to_ptr(void) {
 #endif
 }
 
-int main(int argc, char *argv[]) {
+static void test_ioprio_class_from_to_string_one(const char *val, int expected) {
+        assert_se(ioprio_class_from_string(val) == expected);
+        if (expected >= 0) {
+                _cleanup_free_ char *s = NULL;
+                unsigned ret;
 
+                assert_se(ioprio_class_to_string_alloc(expected, &s) == 0);
+                /* We sometimes get a class number and sometimes a number back */
+                assert_se(streq(s, val) ||
+                          safe_atou(val, &ret) == 0);
+        }
+}
+
+static void test_ioprio_class_from_to_string(void) {
+        test_ioprio_class_from_to_string_one("none", IOPRIO_CLASS_NONE);
+        test_ioprio_class_from_to_string_one("realtime", IOPRIO_CLASS_RT);
+        test_ioprio_class_from_to_string_one("best-effort", IOPRIO_CLASS_BE);
+        test_ioprio_class_from_to_string_one("idle", IOPRIO_CLASS_IDLE);
+        test_ioprio_class_from_to_string_one("0", 0);
+        test_ioprio_class_from_to_string_one("1", 1);
+        test_ioprio_class_from_to_string_one("7", 7);
+        test_ioprio_class_from_to_string_one("8", 8);
+        test_ioprio_class_from_to_string_one("9", -1);
+        test_ioprio_class_from_to_string_one("-1", -1);
+}
+
+int main(int argc, char *argv[]) {
         log_set_max_level(LOG_DEBUG);
         log_parse_environment();
         log_open();
@@ -569,6 +594,7 @@ int main(int argc, char *argv[]) {
         test_getpid_measure();
         test_safe_fork();
         test_pid_to_ptr();
+        test_ioprio_class_from_to_string();
 
         return 0;
 }

src/test/test-socket-util.c

@@ -119,6 +119,16 @@ static void test_socket_address_parse_netlink(void) {
         assert_se(a.sockaddr.sa.sa_family == AF_NETLINK);
         assert_se(a.protocol == NETLINK_ROUTE);
 
+        /* With spaces and tabs */
+        assert_se(socket_address_parse_netlink(&a, " kobject-uevent ") >= 0);
+        assert_se(socket_address_parse_netlink(&a, " \t kobject-uevent \t 10 \t") >= 0);
+        assert_se(a.sockaddr.sa.sa_family == AF_NETLINK);
+        assert_se(a.protocol == NETLINK_KOBJECT_UEVENT);
+
+        assert_se(socket_address_parse_netlink(&a, "kobject-uevent\t10") >= 0);
+        assert_se(a.sockaddr.sa.sa_family == AF_NETLINK);
+        assert_se(a.protocol == NETLINK_KOBJECT_UEVENT);
+
         /* oss-fuzz #6884 */
         assert_se(socket_address_parse_netlink(&a, "\xff") < 0);
 }

test/fuzz-corpus/unit-file/directives.service

@@ -124,7 +124,7 @@ LazyUnmount=
 ListenDatagram=
 ListenFIFO=
 ListenMessageQueue=
-ListenNetlink=
+#ListenNetlink=
 ListenSequentialPacket=
 ListenSpecial=
 ListenStream=

test/fuzz-corpus/unit-file/syslog.socket

@@ -53,7 +53,7 @@ ListenDatagram=1.2.3.4:1234
 ListenSequentialPacket=1.2.3.4:1234
 ListenFIFO=
 ListenSpecial=
-ListenNetlink=
+#ListenNetlink=
 ListenMessageQueue=
 ListenUSBFunction=
 SocketProtocol=udplite

test/fuzz-regressions/.gitattributes

@@ -0,0 +1 @@
+/*/*       -whitespace

test/fuzz-regressions/fuzz-unit-file/oss-fuzz-6892

@@ -0,0 +1,3 @@
+service
+[Service]
+USBFunctionStrings=/

test/fuzz-regressions/fuzz-unit-file/oss-fuzz-6897

@@ -0,0 +1,4 @@
+service
+[Service]
+Slice=%H.slice
+TemporaryFileSystem=%c

test/fuzz-regressions/fuzz-unit-file/oss-fuzz-6897-evverx

@@ -0,0 +1,4 @@
+service
+[Service]
+Slice=abc-def.slice
+TemporaryFileSystem=%c

test/fuzz-regressions/fuzz-unit-file/oss-fuzz-6908

@@ -0,0 +1,3 @@
+socket
+[Socket]
+IOSchedulingClass=531473

test/fuzz-regressions/meson.build

@@ -32,4 +32,9 @@ fuzz_regression_tests = '''
         fuzz-unit-file/oss-fuzz-6884
         fuzz-unit-file/oss-fuzz-6885
         fuzz-unit-file/oss-fuzz-6886
+        fuzz-unit-file/oss-fuzz-6917
+        fuzz-unit-file/oss-fuzz-6892
+        fuzz-unit-file/oss-fuzz-6908
+        fuzz-unit-file/oss-fuzz-6897
+        fuzz-unit-file/oss-fuzz-6897-evverx
 '''.split()