From 52d4d1d339c405818c80960f981ede337785c0da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 14 Mar 2018 14:27:04 +0100 Subject: [PATCH 1/5] test: run all fuzz regression tests with all sanitizers We currently have just one sanitizer for tests, asan, but we may add more in the future. So let's keep the loop over the sanitizers in meson.build, but just enable all regression cases under all sanitizers. If it fails under one of them, it might fail under a different one. In subsequent commits I'll add test cases which might not fail under asan, but it's good to commit them for future use. The test names are made more verbose: 256/257 fuzz-dns-packet:oss-fuzz-5465:address OK 0.04 s 257/257 fuzz-dns-packet:issue-7888:address OK 0.03 s --- meson.build | 92 ++++++++---------- .../{address => }/fuzz-dns-packet/issue-7888 | Bin .../fuzz-dns-packet/oss-fuzz-5465 | Bin test/fuzz-regressions/meson.build | 6 +- 4 files changed, 47 insertions(+), 51 deletions(-) rename test/fuzz-regressions/{address => }/fuzz-dns-packet/issue-7888 (100%) rename test/fuzz-regressions/{address => }/fuzz-dns-packet/oss-fuzz-5465 (100%) diff --git a/meson.build b/meson.build index 56e3d0086e..36869ff046 100644 --- a/meson.build +++ b/meson.build @@ -412,20 +412,6 @@ foreach arg : ['-Wl,-z,relro', endif endforeach -# Check if various sanitizers are supported -sanitizers = [] -foreach arg : ['address'] - - have = run_command(check_compilation_sh, - cc.cmd_array(), '-x', 'c', - '-fsanitize=@0@'.format(arg), - '-include', link_test_c).returncode() == 0 - message('@0@ sanitizer supported: @1@'.format(arg, have ? 'yes' : 'no')) - if have - sanitizers += arg - endif -endforeach - if get_option('buildtype') != 'debug' foreach arg : ['-ffunction-sections', '-fdata-sections'] @@ -2657,48 +2643,56 @@ endforeach ############################################################ -prev = '' -foreach p : fuzz_regression_tests - a = p.split('/')[-3] - b = p.split('/')[-2] - c = p.split('/')[-1] +# Enable tests for all supported sanitizers +foreach tuple : sanitizers + sanitizer = tuple[0] + build = tuple[1] - if a == 'address' - build = sanitize_address - else - error('unknown sanitizer @0@'.format(a)) - endif + have = run_command(check_compilation_sh, + cc.cmd_array(), '-x', 'c', + '-fsanitize=@0@'.format(sanitizer), + '-include', link_test_c).returncode() == 0 + message('@0@ sanitizer supported: @1@'.format(sanitizer, have ? 'yes' : 'no')) - name = '@1@:@0@'.format(a, b) + if have + prev = '' + foreach p : fuzz_regression_tests + b = p.split('/')[-2] + c = p.split('/')[-1] - if name != prev - if want_tests == 'false' - message('Not compiling @0@ because tests is set to false'.format(name)) - elif not sanitizers.contains(a) - message('Not compiling @0@ because @1@ sanitizer is not available'.format(name, a)) - elif slow_tests - exe = custom_target( - name, - output : name, - depends : build, - command : [env, 'ln', '-fs', - join_paths(build.full_path(), b), - '@OUTPUT@'], - build_by_default : true) - else - message('Not compiling @0@ because slow-tests is set to false'.format(name)) - endif - endif - prev = name + name = '@0@:@1@'.format(b, sanitizer) - if want_tests != 'false' and slow_tests - test(c, env, args : [exe.full_path(), - join_paths(meson.source_root(), - 'test/fuzz-regressions', - p)]) + if name != prev + if want_tests == 'false' + message('Not compiling @0@ because tests is set to false'.format(name)) + elif slow_tests + exe = custom_target( + name, + output : name, + depends : build, + command : [env, 'ln', '-fs', + join_paths(build.full_path(), b), + '@OUTPUT@'], + build_by_default : true) + else + message('Not compiling @0@ because slow-tests is set to false'.format(name)) + endif + endif + prev = name + + if want_tests != 'false' and slow_tests + test('@0@:@1@:@2@'.format(b, c, sanitizer), + env, + args : [exe.full_path(), + join_paths(meson.source_root(), + 'test/fuzz-regressions', + p)]) + endif + endforeach endif endforeach + ############################################################ if git.found() diff --git a/test/fuzz-regressions/address/fuzz-dns-packet/issue-7888 b/test/fuzz-regressions/fuzz-dns-packet/issue-7888 similarity index 100% rename from test/fuzz-regressions/address/fuzz-dns-packet/issue-7888 rename to test/fuzz-regressions/fuzz-dns-packet/issue-7888 diff --git a/test/fuzz-regressions/address/fuzz-dns-packet/oss-fuzz-5465 b/test/fuzz-regressions/fuzz-dns-packet/oss-fuzz-5465 similarity index 100% rename from test/fuzz-regressions/address/fuzz-dns-packet/oss-fuzz-5465 rename to test/fuzz-regressions/fuzz-dns-packet/oss-fuzz-5465 diff --git a/test/fuzz-regressions/meson.build b/test/fuzz-regressions/meson.build index de69c941ea..25ec51148b 100644 --- a/test/fuzz-regressions/meson.build +++ b/test/fuzz-regressions/meson.build @@ -24,7 +24,9 @@ sanitize_address = custom_target( 'fuzzers', '-Db_lundef=false -Db_sanitize=address']) +sanitizers = [['address', sanitize_address]] + fuzz_regression_tests = ''' - address/fuzz-dns-packet/oss-fuzz-5465 - address/fuzz-dns-packet/issue-7888 + fuzz-dns-packet/oss-fuzz-5465 + fuzz-dns-packet/issue-7888 '''.split() From 8d89e51f3c58e3712765e7a9e1cddf45d3594509 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 14 Mar 2018 14:31:24 +0100 Subject: [PATCH 2/5] fuzz: commit test case for oss-fuzz issue 6884 This seems to be a false positive in msan: https://github.com/google/sanitizers/issues/767. I don't see anything wrong with the code either, and valgrind does not see the issue. Anyway, let's add the test case. We don't have msan hooked up yet, but hopefully we'll in the future. oss-fuzz #6884. --- src/test/test-socket-util.c | 3 +++ test/fuzz-regressions/fuzz-unit-file/oss-fuzz-6884 | 3 +++ test/fuzz-regressions/meson.build | 1 + 3 files changed, 7 insertions(+) create mode 100644 test/fuzz-regressions/fuzz-unit-file/oss-fuzz-6884 diff --git a/src/test/test-socket-util.c b/src/test/test-socket-util.c index e35a27fa61..76896b0328 100644 --- a/src/test/test-socket-util.c +++ b/src/test/test-socket-util.c @@ -118,6 +118,9 @@ static void test_socket_address_parse_netlink(void) { assert_se(socket_address_parse_netlink(&a, "route 10") >= 0); assert_se(a.sockaddr.sa.sa_family == AF_NETLINK); assert_se(a.protocol == NETLINK_ROUTE); + + /* oss-fuzz #6884 */ + assert_se(socket_address_parse_netlink(&a, "\xff") < 0); } static void test_socket_address_equal(void) { diff --git a/test/fuzz-regressions/fuzz-unit-file/oss-fuzz-6884 b/test/fuzz-regressions/fuzz-unit-file/oss-fuzz-6884 new file mode 100644 index 0000000000..00d105ade5 --- /dev/null +++ b/test/fuzz-regressions/fuzz-unit-file/oss-fuzz-6884 @@ -0,0 +1,3 @@ +socket +[Socket] +ListenNetlink=ÿ \ No newline at end of file diff --git a/test/fuzz-regressions/meson.build b/test/fuzz-regressions/meson.build index 25ec51148b..c1416f93cf 100644 --- a/test/fuzz-regressions/meson.build +++ b/test/fuzz-regressions/meson.build @@ -29,4 +29,5 @@ sanitizers = [['address', sanitize_address]] fuzz_regression_tests = ''' fuzz-dns-packet/oss-fuzz-5465 fuzz-dns-packet/issue-7888 + fuzz-unit-file/oss-fuzz-6884 '''.split() From 0d032da9935b1511d46db7afdc95c139470fc9f0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 13 Mar 2018 12:22:59 +0100 Subject: [PATCH 3/5] TODO: trim obsolete entries set -e is now used in test/TEST-*/test.sh, BUILD_DIR may be specified, and symlinks are created as of ba7f4ae6178309dc937e10cf7dce0eca9dafb8de. --- TODO | 4 ---- 1 file changed, 4 deletions(-) diff --git a/TODO b/TODO index 18acb9b363..f171121f69 100644 --- a/TODO +++ b/TODO @@ -528,8 +528,6 @@ Features: * maybe add a generator that looks for "systemd.run=" on the kernel cmdline for container usercases... * test/: - - add 'set -e' to scripts in test/ - - make stuff in test/ work with separate output dir * seems that when we follow symlinks to units we prefer the symlink destination path over /etc and /usr. We should not do that. Instead @@ -774,8 +772,6 @@ Features: * hw watchdog: optionally try to use the preset watchdog timeout instead of always overriding it https://bugs.freedesktop.org/show_bug.cgi?id=54712 -* create /sbin/init symlinks from the build system - * add a dependency on standard-conf.xml and other included files to man pages * MountFlags=shared acts as MountFlags=slave right now. From 20d52ab60e7ba40f7cf23c148bcead8bd05bea3a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 13 Mar 2018 12:25:06 +0100 Subject: [PATCH 4/5] shared/conf-parser: fix crash when specifiers cannot be resolved in config_parse_device_allow() oss-fuzz #6885. --- TODO | 1 + src/core/load-fragment.c | 2 +- test/fuzz-regressions/fuzz-unit-file/oss-fuzz-6885 | 3 +++ test/fuzz-regressions/meson.build | 1 + 4 files changed, 6 insertions(+), 1 deletion(-) create mode 100644 test/fuzz-regressions/fuzz-unit-file/oss-fuzz-6885 diff --git a/TODO b/TODO index f171121f69..d5e37e49ac 100644 --- a/TODO +++ b/TODO @@ -528,6 +528,7 @@ Features: * maybe add a generator that looks for "systemd.run=" on the kernel cmdline for container usercases... * test/: + - add unit tests for config_parse_device_allow() * seems that when we follow symlinks to units we prefer the symlink destination path over /etc and /usr. We should not do that. Instead diff --git a/src/core/load-fragment.c b/src/core/load-fragment.c index f2f9267b92..7f56149ead 100644 --- a/src/core/load-fragment.c +++ b/src/core/load-fragment.c @@ -410,7 +410,6 @@ int config_parse_socket_listen(const char *unit, if (r < 0) { if (r != -EAFNOSUPPORT) log_syntax(unit, LOG_ERR, filename, line, r, "Failed to parse address value, ignoring: %s", rvalue); - return 0; } @@ -3511,6 +3510,7 @@ int config_parse_device_allow( log_syntax(unit, LOG_WARNING, filename, line, r, "Failed to resolve specifiers in %s, ignoring: %m", rvalue); + return 0; } n = strcspn(t, WHITESPACE); diff --git a/test/fuzz-regressions/fuzz-unit-file/oss-fuzz-6885 b/test/fuzz-regressions/fuzz-unit-file/oss-fuzz-6885 new file mode 100644 index 0000000000..1859136fdc --- /dev/null +++ b/test/fuzz-regressions/fuzz-unit-file/oss-fuzz-6885 @@ -0,0 +1,3 @@ +service +[Service] +DeviceAllow=%D \ No newline at end of file diff --git a/test/fuzz-regressions/meson.build b/test/fuzz-regressions/meson.build index c1416f93cf..9753c61882 100644 --- a/test/fuzz-regressions/meson.build +++ b/test/fuzz-regressions/meson.build @@ -30,4 +30,5 @@ fuzz_regression_tests = ''' fuzz-dns-packet/oss-fuzz-5465 fuzz-dns-packet/issue-7888 fuzz-unit-file/oss-fuzz-6884 + fuzz-unit-file/oss-fuzz-6885 '''.split() From 55a30fd4e832891604b1775cbfb06a85d52d9424 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 13 Mar 2018 12:51:08 +0100 Subject: [PATCH 5/5] basic/calendarspec: fix assert crash when year is too large in calendarspec_from_time_t() gmtime_r() will return NULL in that case, and we would crash. I committed the reproducer case in fuzz-regressions/, even though we don't have ubsan hooked up yet. Let's add it anyway in case it is useful in the future. We actually crash anyway when compiled with asserts, so this can be easily reproduced without ubsan. oss-fuzz #6886. --- src/basic/calendarspec.c | 3 ++- test/fuzz-regressions/fuzz-unit-file/oss-fuzz-6886 | 3 +++ test/fuzz-regressions/meson.build | 1 + 3 files changed, 6 insertions(+), 1 deletion(-) create mode 100644 test/fuzz-regressions/fuzz-unit-file/oss-fuzz-6886 diff --git a/src/basic/calendarspec.c b/src/basic/calendarspec.c index fd78022773..3918428a57 100644 --- a/src/basic/calendarspec.c +++ b/src/basic/calendarspec.c @@ -581,7 +581,8 @@ static int calendarspec_from_time_t(CalendarSpec *c, time_t time) { CalendarComponent *year = NULL, *month = NULL, *day = NULL, *hour = NULL, *minute = NULL, *us = NULL; int r; - assert_se(gmtime_r(&time, &tm)); + if (!gmtime_r(&time, &tm)) + return -ERANGE; r = const_chain(tm.tm_year + 1900, &year); if (r < 0) diff --git a/test/fuzz-regressions/fuzz-unit-file/oss-fuzz-6886 b/test/fuzz-regressions/fuzz-unit-file/oss-fuzz-6886 new file mode 100644 index 0000000000..1fbe5ffd99 --- /dev/null +++ b/test/fuzz-regressions/fuzz-unit-file/oss-fuzz-6886 @@ -0,0 +1,3 @@ +timer +[Timer] +OnCalendar=@88588582097858858 \ No newline at end of file diff --git a/test/fuzz-regressions/meson.build b/test/fuzz-regressions/meson.build index 9753c61882..ee00bcd046 100644 --- a/test/fuzz-regressions/meson.build +++ b/test/fuzz-regressions/meson.build @@ -31,4 +31,5 @@ fuzz_regression_tests = ''' fuzz-dns-packet/issue-7888 fuzz-unit-file/oss-fuzz-6884 fuzz-unit-file/oss-fuzz-6885 + fuzz-unit-file/oss-fuzz-6886 '''.split()