From 41dceb91dd10fd597d3a26adcb245a18e0c2c077 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sat, 3 Sep 2022 23:10:24 +0900 Subject: [PATCH 1/5] json: introduce json_append() --- src/shared/json.c | 24 ++++++++++++++++++++++++ src/shared/json.h | 3 +++ src/test/test-json.c | 15 +++++++++++++++ 3 files changed, 42 insertions(+) diff --git a/src/shared/json.c b/src/shared/json.c index f0416e66dc..d541eb1036 100644 --- a/src/shared/json.c +++ b/src/shared/json.c @@ -4120,6 +4120,30 @@ int json_build(JsonVariant **ret, ...) { return r; } +int json_appendv(JsonVariant **v, va_list ap) { + _cleanup_(json_variant_unrefp) JsonVariant *w = NULL; + int r; + + assert(v); + + r = json_buildv(&w, ap); + if (r < 0) + return r; + + return json_variant_merge(v, w); +} + +int json_append(JsonVariant **v, ...) { + va_list ap; + int r; + + va_start(ap, v); + r = json_appendv(v, ap); + va_end(ap); + + return r; +} + int json_log_internal( JsonVariant *variant, int level, diff --git a/src/shared/json.h b/src/shared/json.h index 98d184c309..c75dfe8741 100644 --- a/src/shared/json.h +++ b/src/shared/json.h @@ -327,6 +327,9 @@ enum { int json_build(JsonVariant **ret, ...); int json_buildv(JsonVariant **ret, va_list ap); + /* These two functions below are equivalent to json_build() (or json_buildv()) and json_variant_merge(). */ +int json_append(JsonVariant **v, ...); +int json_appendv(JsonVariant **v, va_list ap); /* A bitmask of flags used by the dispatch logic. Note that this is a combined bit mask, that is generated from the bit * mask originally passed into json_dispatch(), the individual bitmask associated with the static JsonDispatch callout diff --git a/src/test/test-json.c b/src/test/test-json.c index d22485630a..3563d004c8 100644 --- a/src/test/test-json.c +++ b/src/test/test-json.c @@ -631,4 +631,19 @@ TEST(variant) { test_variant_one("[ 0, -0, 0.0, -0.0, 0.000, -0.000, 0e0, -0e0, 0e+0, -0e-0, 0e-0, -0e000, 0e+000 ]", test_zeroes); } +TEST(json_append) { + _cleanup_(json_variant_unrefp) JsonVariant *v = NULL, *w = NULL; + + assert_se(json_build(&v, JSON_BUILD_OBJECT( + JSON_BUILD_PAIR("b", JSON_BUILD_STRING("x")), + JSON_BUILD_PAIR("c", JSON_BUILD_CONST_STRING("y")), + JSON_BUILD_PAIR("a", JSON_BUILD_CONST_STRING("z")))) >= 0); + + assert_se(json_append(&w, JSON_BUILD_OBJECT(JSON_BUILD_PAIR("b", JSON_BUILD_STRING("x")))) >= 0); + assert_se(json_append(&w, JSON_BUILD_OBJECT(JSON_BUILD_PAIR("c", JSON_BUILD_STRING("y")))) >= 0); + assert_se(json_append(&w, JSON_BUILD_OBJECT(JSON_BUILD_PAIR("a", JSON_BUILD_STRING("z")))) >= 0); + + assert_se(json_variant_equal(v, w)); +} + DEFINE_TEST_MAIN(LOG_DEBUG); From 4d49d19e9d30a5173b9f375051a8f547319ffed9 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sat, 3 Sep 2022 22:01:16 +0900 Subject: [PATCH 2/5] bootspec: do not build two many json object at once This is a workaround for an issue in the memory sanitizer. If a function is called with too many arguments, then the sanitizer triggers the following false-positive warning: ==349==WARNING: MemorySanitizer: use-of-uninitialized-value #0 0x7f8b247134a7 in json_buildv /work/build/../../src/systemd/src/shared/json.c:3213:17 #1 0x7f8b24714231 in json_build /work/build/../../src/systemd/src/shared/json.c:4117:13 #2 0x7f8b24487fa5 in show_boot_entries /work/build/../../src/systemd/src/shared/bootspec.c:1424:29 #3 0x4a6a1b in LLVMFuzzerTestOneInput /work/build/../../src/systemd/src/fuzz/fuzz-bootspec.c:119:16 #4 0x4c6693 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:611:15 #5 0x4c5e7a in fuzzer::Fuzzer::RunOne(unsigned char const*, unsigned long, bool, fuzzer::InputInfo*, bool, bool*) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:514:3 #6 0x4c7ce4 in fuzzer::Fuzzer::ReadAndExecuteSeedCorpora(std::__Fuzzer::vector >&) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:826:7 #7 0x4c7f19 in fuzzer::Fuzzer::Loop(std::__Fuzzer::vector >&) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:857:3 #8 0x4b757f in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:912:6 #9 0x4e0bd2 in main /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerMain.cpp:20:10 #10 0x7f8b23ead082 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x24082) (BuildId: 1878e6b475720c7c51969e69ab2d276fae6d1dee) #11 0x41f69d in _start (build-out/fuzz-bootspec+0x41f69d) Follow-up for #24541. Fixes #24551. --- src/shared/bootspec.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/shared/bootspec.c b/src/shared/bootspec.c index 332b5cfed3..8927f750fd 100644 --- a/src/shared/bootspec.c +++ b/src/shared/bootspec.c @@ -1421,7 +1421,7 @@ int show_boot_entries(const BootConfig *config, JsonFormatFlags json_format) { return log_oom(); } - r = json_build(&v, JSON_BUILD_OBJECT( + r = json_append(&v, JSON_BUILD_OBJECT( JSON_BUILD_PAIR_CONDITION(e->id, "id", JSON_BUILD_STRING(e->id)), JSON_BUILD_PAIR_CONDITION(e->path, "path", JSON_BUILD_STRING(e->path)), JSON_BUILD_PAIR_CONDITION(e->root, "root", JSON_BUILD_STRING(e->root)), @@ -1436,7 +1436,14 @@ int show_boot_entries(const BootConfig *config, JsonFormatFlags json_format) { JSON_BUILD_PAIR_CONDITION(e->efi, "efi", JSON_BUILD_STRING(e->efi)), JSON_BUILD_PAIR_CONDITION(!strv_isempty(e->initrd), "initrd", JSON_BUILD_STRV(e->initrd)), JSON_BUILD_PAIR_CONDITION(e->device_tree, "devicetree", JSON_BUILD_STRING(e->device_tree)), - JSON_BUILD_PAIR_CONDITION(!strv_isempty(e->device_tree_overlay), "devicetreeOverlay", JSON_BUILD_STRV(e->device_tree_overlay)), + JSON_BUILD_PAIR_CONDITION(!strv_isempty(e->device_tree_overlay), "devicetreeOverlay", JSON_BUILD_STRV(e->device_tree_overlay)))); + if (r < 0) + return log_oom(); + + /* Sanitizers (only memory sanitizer?) do not like function call with too many + * arguments and trigger false positive warnings. Let's not add too many json objects + * at once. */ + r = json_append(&v, JSON_BUILD_OBJECT( JSON_BUILD_PAIR_CONDITION(e->tries_left != UINT_MAX, "triesLeft", JSON_BUILD_UNSIGNED(e->tries_left)), JSON_BUILD_PAIR_CONDITION(e->tries_done != UINT_MAX, "triesDone", JSON_BUILD_UNSIGNED(e->tries_done)))); if (r < 0) From 69a34a4fd4202a4917d7eddb60ae6df6bc9562c0 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sat, 3 Sep 2022 23:21:18 +0900 Subject: [PATCH 3/5] fuzz: add a test case for fuzz-bootspec This adds a testcase for the issue oss-fuzz#50949 (https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=50949). --- ...clusterfuzz-testcase-minimized-fuzz-bootspec-5421682902302720 | 1 + 1 file changed, 1 insertion(+) create mode 100644 test/fuzz/fuzz-bootspec/clusterfuzz-testcase-minimized-fuzz-bootspec-5421682902302720 diff --git a/test/fuzz/fuzz-bootspec/clusterfuzz-testcase-minimized-fuzz-bootspec-5421682902302720 b/test/fuzz/fuzz-bootspec/clusterfuzz-testcase-minimized-fuzz-bootspec-5421682902302720 new file mode 100644 index 0000000000..b95df60504 --- /dev/null +++ b/test/fuzz/fuzz-bootspec/clusterfuzz-testcase-minimized-fuzz-bootspec-5421682902302720 @@ -0,0 +1 @@ +{"loader":[""]} \ No newline at end of file From 28340719f27b805d5cb6dfbdf4284de8f6880ba7 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sun, 4 Sep 2022 00:01:09 +0900 Subject: [PATCH 4/5] bootspec: shorten code a bit --- src/shared/bootspec.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/shared/bootspec.c b/src/shared/bootspec.c index 8927f750fd..33f2ea6652 100644 --- a/src/shared/bootspec.c +++ b/src/shared/bootspec.c @@ -113,8 +113,7 @@ static int parse_path_one( if (r <= 0) return r; - free_and_replace(*s, c); - return 0; + return free_and_replace(*s, c); } static int parse_path_strv( From b488c59c9fa06e7ec4108469f042e6f8d3751db2 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sun, 4 Sep 2022 00:02:38 +0900 Subject: [PATCH 5/5] test: check returned values are always initialized on success --- src/test/test-bootspec.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/test/test-bootspec.c b/src/test/test-bootspec.c index 3c645a3e79..67fa8beea9 100644 --- a/src/test/test-bootspec.c +++ b/src/test/test-bootspec.c @@ -95,9 +95,12 @@ TEST_RET(bootspec_sort) { static void test_extract_tries_one(const char *fname, int ret, const char *stripped, unsigned tries_left, unsigned tries_done) { _cleanup_free_ char *p = NULL; - unsigned l = UINT_MAX, d = UINT_MAX; + unsigned l, d; assert_se(boot_filename_extract_tries(fname, &p, &l, &d) == ret); + if (ret < 0) + return; + assert_se(streq_ptr(p, stripped)); assert_se(l == tries_left); assert_se(d == tries_done);