Merge pull request #11796 from yuwata/fuzz-link-parser

fuzz: add fuzzer for .link files

src/network/fuzz-netdev-parser.c

@@ -18,7 +18,7 @@ int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) {
         if (size != 0)
                 assert_se(fwrite(data, size, 1, f) == 1);
 
-        rewind(f);
+        fflush(f);
         assert_se(manager_new(&manager) >= 0);
         (void) netdev_load_one(manager, netdev_config);
         return 0;

src/network/fuzz-network-parser.c

@@ -18,7 +18,7 @@ int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) {
         if (size != 0)
                 assert_se(fwrite(data, size, 1, f) == 1);
 
-        rewind(f);
+        fflush(f);
         assert_se(manager_new(&manager) >= 0);
         (void) network_load_one(manager, network_config);
         return 0;

src/udev/meson.build

@@ -197,3 +197,14 @@ configure_file(
 
 meson.add_install_script('sh', '-c',
                          mkdir_p.format(join_paths(sysconfdir, 'udev/rules.d')))
+
+fuzzers += [
+        [['src/udev/net/fuzz-link-parser.c',
+          'src/fuzz/fuzz.h'],
+         [libudev_core,
+          libudev_static,
+          libsystemd_network,
+          libshared],
+         [threads,
+          libacl]]
+        ]

src/udev/net/ethtool-util.c

@@ -775,7 +775,9 @@ int config_parse_advertise(const char *unit,
                         break;
 
                 mode = ethtool_link_mode_bit_from_string(w);
-                if (mode < 0) {
+                /* We reuse the kernel provided enum which does not contain negative value. So, the cast
+                 * below is mandatory. Otherwise, the check below always passes and access an invalid address. */
+                if ((int) mode < 0) {
                         log_syntax(unit, LOG_ERR, filename, line, 0, "Failed to parse advertise mode, ignoring: %s", w);
                         continue;
                 }

src/udev/net/fuzz-link-parser.c

@@ -0,0 +1,25 @@
+/* SPDX-License-Identifier: LGPL-2.1+ */
+
+#include "fd-util.h"
+#include "fs-util.h"
+#include "fuzz.h"
+#include "link-config.h"
+#include "tmpfile-util.h"
+
+int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) {
+        _cleanup_(link_config_ctx_freep) link_config_ctx *ctx = NULL;
+        _cleanup_(unlink_tempfilep) char filename[] = "/tmp/fuzz-link-config.XXXXXX";
+        _cleanup_fclose_ FILE *f = NULL;
+
+        if (!getenv("SYSTEMD_LOG_LEVEL"))
+                log_set_max_level(LOG_CRIT);
+
+        assert_se(fmkostemp_safe(filename, "r+", &f) == 0);
+        if (size != 0)
+                assert_se(fwrite(data, size, 1, f) == 1);
+
+        fflush(f);
+        assert_se(link_config_ctx_new(&ctx) >= 0);
+        (void) link_load_one(ctx, filename);
+        return 0;
+}

src/udev/net/link-config.c

@@ -50,12 +50,13 @@ static void link_config_free(link_config *link) {
         strv_free(link->match_path);
         strv_free(link->match_driver);
         strv_free(link->match_type);
-        free(link->match_name);
-        free(link->match_host);
-        free(link->match_virt);
-        free(link->match_kernel_cmdline);
-        free(link->match_kernel_version);
-        free(link->match_arch);
+        strv_free(link->match_name);
+
+        condition_free_list(link->match_host);
+        condition_free_list(link->match_virt);
+        condition_free_list(link->match_kernel_cmdline);
+        condition_free_list(link->match_kernel_version);
+        condition_free_list(link->match_arch);
 
         free(link->description);
         free(link->mac);
@@ -93,8 +94,6 @@ void link_config_ctx_free(link_config_ctx *ctx) {
         return;
 }
 
-DEFINE_TRIVIAL_CLEANUP_FUNC(link_config_ctx*, link_config_ctx_free);
-
 int link_config_ctx_new(link_config_ctx **ret) {
         _cleanup_(link_config_ctx_freep) link_config_ctx *ctx = NULL;
 
@@ -116,56 +115,64 @@ int link_config_ctx_new(link_config_ctx **ret) {
         return 0;
 }
 
-static int load_link(link_config_ctx *ctx, const char *filename) {
+int link_load_one(link_config_ctx *ctx, const char *filename) {
         _cleanup_(link_config_freep) link_config *link = NULL;
         _cleanup_fclose_ FILE *file = NULL;
-        int i;
+        _cleanup_free_ char *name = NULL;
+        size_t i;
         int r;
 
         assert(ctx);
         assert(filename);
 
         file = fopen(filename, "re");
-        if (!file) {
-                if (errno == ENOENT)
-                        return 0;
-                else
-                        return -errno;
-        }
+        if (!file)
+                return errno == ENOENT ? 0 : -errno;
 
         if (null_or_empty_fd(fileno(file))) {
                 log_debug("Skipping empty file: %s", filename);
                 return 0;
         }
 
-        link = new0(link_config, 1);
+        name = strdup(filename);
+        if (!name)
+                return -ENOMEM;
+
+        link = new(link_config, 1);
         if (!link)
-                return log_oom();
+                return -ENOMEM;
 
-        link->mac_policy = _MACPOLICY_INVALID;
-        link->wol = _WOL_INVALID;
-        link->duplex = _DUP_INVALID;
-        link->port = _NET_DEV_PORT_INVALID;
-        link->autonegotiation = -1;
+        *link = (link_config) {
+                .filename = TAKE_PTR(name),
+                .mac_policy = _MACPOLICY_INVALID,
+                .wol = _WOL_INVALID,
+                .duplex = _DUP_INVALID,
+                .port = _NET_DEV_PORT_INVALID,
+                .autonegotiation = -1,
+        };
 
-        for (i = 0; i < (int)ELEMENTSOF(link->features); i++)
+        for (i = 0; i < ELEMENTSOF(link->features); i++)
                 link->features[i] = -1;
 
         r = config_parse(NULL, filename, file,
-                         "Match\0Link\0Ethernet\0",
+                         "Match\0Link\0",
                          config_item_perf_lookup, link_config_gperf_lookup,
                          CONFIG_PARSE_WARN, link);
         if (r < 0)
                 return r;
-        else
-                log_debug("Parsed configuration file %s", filename);
 
         if (link->speed > UINT_MAX)
                 return -ERANGE;
 
-        link->filename = strdup(filename);
-        if (!link->filename)
-                return log_oom();
+        if (!net_match_config(NULL, NULL, NULL, NULL, NULL,
+                              link->match_host, link->match_virt, link->match_kernel_cmdline,
+                              link->match_kernel_version, link->match_arch,
+                              NULL, NULL, NULL, NULL, NULL)) {
+                log_debug("%s: Conditions do not match the system environment, skipping.", filename);
+                return 0;
+        }
+
+        log_debug("Parsed configuration file %s", filename);
 
         LIST_PREPEND(links, ctx->links, link);
         link = NULL;
@@ -215,9 +222,9 @@ int link_config_load(link_config_ctx *ctx) {
                 return log_error_errno(r, "failed to enumerate link files: %m");
 
         STRV_FOREACH_BACKWARDS(f, files) {
-                r = load_link(ctx, *f);
+                r = link_load_one(ctx, *f);
                 if (r < 0)
-                        return r;
+                        log_error_errno(r, "Failed to load %s, ignoring: %m", *f);
         }
 
         return 0;

src/udev/net/link-config.h

@@ -67,7 +67,9 @@ struct link_config {
 
 int link_config_ctx_new(link_config_ctx **ret);
 void link_config_ctx_free(link_config_ctx *ctx);
+DEFINE_TRIVIAL_CLEANUP_FUNC(link_config_ctx*, link_config_ctx_free);
 
+int link_load_one(link_config_ctx *ctx, const char *filename);
 int link_config_load(link_config_ctx *ctx);
 bool link_config_should_reload(link_config_ctx *ctx);
 

test/fuzz/fuzz-link-parser/99-default.link

@@ -0,0 +1,12 @@
+#  SPDX-License-Identifier: LGPL-2.1+
+#
+#  This file is part of systemd.
+#
+#  systemd is free software; you can redistribute it and/or modify it
+#  under the terms of the GNU Lesser General Public License as published by
+#  the Free Software Foundation; either version 2.1 of the License, or
+#  (at your option) any later version.
+
+[Link]
+NamePolicy=keep kernel database onboard slot path
+MACAddressPolicy=persistent

test/fuzz/fuzz-link-parser/advertise-segv.link

@@ -0,0 +1,2 @@
+[Link]
+Advertise=hoge foo

test/fuzz/fuzz-link-parser/condition-memleak.link

@@ -0,0 +1,7 @@
+[Match]
+OriginalName=eth0
+Host=hogehoge
+Virtualization=qemu
+KernelCommandLine=aaa
+KernelVersion=4.20.3
+Architecture=x86-64

test/fuzz/fuzz-link-parser/directives.link

@@ -0,0 +1,35 @@
+[Match]
+MACAddress=
+OriginalName=
+Path=
+Driver=
+Type=
+Host=
+Virtualization=
+KernelCommandLine=
+KernelVersion=
+Architecture=
+[Link]
+Description=
+MACAddressPolicy=
+MACAddress=
+NamePolicy=
+Name=
+Alias=
+MTUBytes=
+BitsPerSecond=
+Duplex=
+AutoNegotiation=
+WakeOnLan=
+Port=
+GenericSegmentationOffload=
+TCPSegmentationOffload=
+TCP6SegmentationOffload=
+UDPSegmentationOffload=
+GenericReceiveOffload=
+LargeReceiveOffload=
+RxChannels=
+TxChannels=
+OtherChannels=
+CombinedChannels=
+Advertise=

tools/check-directives.sh

@@ -10,12 +10,20 @@ function generate_directives() {
         }}' "$1"
 }
 
+ret=0
 if [[ $(generate_directives src/network/networkd-network-gperf.gperf | wc -l) -ne $(wc -l <test/fuzz/fuzz-network-parser/directives.network) ]]; then
 	echo "Looks like test/fuzz/fuzz-network-parser/directives.network hasn't been updated"
-	exit 1
+        ret=1
 fi
 
 if [[ $(generate_directives src/network/netdev/netdev-gperf.gperf | wc -l) -ne $(wc -l <test/fuzz/fuzz-netdev-parser/directives.netdev) ]]; then
 	echo "Looks like test/fuzz/fuzz-netdev-parser/directives.netdev hasn't been updated"
-	exit 1
+	ret=1
 fi
+
+if [[ $(generate_directives src/udev/net/link-config-gperf.gperf | wc -l) -ne $(wc -l <test/fuzz/fuzz-link-parser/directives.link) ]]; then
+	echo "Looks like test/fuzz/fuzz-link-parser/directives.link hasn't been updated"
+	ret=1
+fi
+
+exit $ret