From ac874b8fb13bf293986a3814149a820729b27a30 Mon Sep 17 00:00:00 2001 From: Frantisek Sumsal Date: Tue, 23 May 2023 18:09:23 +0200 Subject: [PATCH 1/7] sd-journal: avoid double-free If we fail to combine the new entry with a previous one, or update it in the hashmap, we might later on attempt a double-free: ================================================================= ==10==ERROR: AddressSanitizer: attempting double-free on 0x611000039fc0 in thread T0: SCARINESS: 42 (double-free) #0 0x4a0962 in __interceptor_free /src/llvm-project/compiler-rt/lib/asan/asan_malloc_linux.cpp:52:3 #1 0x7f55e431d9f2 in _hashmap_clear /work/build/../../src/systemd/src/basic/hashmap.c:927:33 #2 0x7f55e431d4c8 in _hashmap_free /work/build/../../src/systemd/src/basic/hashmap.c:896:17 #3 0x4de1de in ordered_hashmap_free_free_free /work/build/../../src/systemd/src/basic/hashmap.h:120:24 #4 0x4de1de in ordered_hashmap_free_free_freep /work/build/../../src/systemd/src/basic/hashmap.h:434:1 #5 0x4de1de in LLVMFuzzerTestOneInput /work/build/../../src/systemd/src/fuzz/fuzz-catalog.c:26:1 #6 0x4de8b8 in NaloFuzzerTestOneInput (/build/fuzz-catalog+0x4de8b8) #7 0x4fd8c3 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:611:15 #8 0x4fd0aa 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 #9 0x4fe779 in fuzzer::Fuzzer::MutateAndTestOne() /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:757:19 #10 0x4ff445 in fuzzer::Fuzzer::Loop(std::__Fuzzer::vector >&) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:895:5 #11 0x4ee7af in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:912:6 #12 0x4ef078 in LLVMFuzzerRunDriver /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:925:10 #13 0x4deb35 in main (/build/fuzz-catalog+0x4deb35) #14 0x7f55e3a32082 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x24082) (BuildId: 1878e6b475720c7c51969e69ab2d276fae6d1dee) #15 0x41f7cd in _start (/build/fuzz-catalog+0x41f7cd) DEDUP_TOKEN: __interceptor_free--_hashmap_clear--_hashmap_free 0x611000039fc0 is located 0 bytes inside of 224-byte region [0x611000039fc0,0x61100003a0a0) freed by thread T0 here: #0 0x4a0962 in __interceptor_free /src/llvm-project/compiler-rt/lib/asan/asan_malloc_linux.cpp:52:3 #1 0x7f55e451493d in freep /work/build/../../src/systemd/src/basic/alloc-util.h:107:22 #2 0x7f55e451493d in finish_item /work/build/../../src/systemd/src/libsystemd/sd-journal/catalog.c:187:1 #3 0x7f55e4513e56 in catalog_import_file /work/build/../../src/systemd/src/libsystemd/sd-journal/catalog.c:313:45 #4 0x4de1be in LLVMFuzzerTestOneInput /work/build/../../src/systemd/src/fuzz/fuzz-catalog.c:23:16 #5 0x4de8b8 in NaloFuzzerTestOneInput (/build/fuzz-catalog+0x4de8b8) #6 0x4fd8c3 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:611:15 #7 0x4fd0aa 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 #8 0x4fe779 in fuzzer::Fuzzer::MutateAndTestOne() /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:757:19 #9 0x4ff445 in fuzzer::Fuzzer::Loop(std::__Fuzzer::vector >&) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:895:5 #10 0x4ee7af in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:912:6 #11 0x4ef078 in LLVMFuzzerRunDriver /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:925:10 #12 0x4deb35 in main (/build/fuzz-catalog+0x4deb35) #13 0x7f55e3a32082 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x24082) (BuildId: 1878e6b475720c7c51969e69ab2d276fae6d1dee) DEDUP_TOKEN: __interceptor_free--freep--finish_item previously allocated by thread T0 here: #0 0x4a0c06 in __interceptor_malloc /src/llvm-project/compiler-rt/lib/asan/asan_malloc_linux.cpp:69:3 #1 0x4de539 in malloc (/build/fuzz-catalog+0x4de539) #2 0x7f55e42bf96b in memdup /work/build/../../src/systemd/src/basic/alloc-util.c:16:15 #3 0x7f55e451475d in finish_item /work/build/../../src/systemd/src/libsystemd/sd-journal/catalog.c:176:28 #4 0x7f55e4513e56 in catalog_import_file /work/build/../../src/systemd/src/libsystemd/sd-journal/catalog.c:313:45 #5 0x4de1be in LLVMFuzzerTestOneInput /work/build/../../src/systemd/src/fuzz/fuzz-catalog.c:23:16 #6 0x4de8b8 in NaloFuzzerTestOneInput (/build/fuzz-catalog+0x4de8b8) #7 0x4fd8c3 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:611:15 #8 0x4fd0aa 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 #9 0x4fe779 in fuzzer::Fuzzer::MutateAndTestOne() /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:757:19 #10 0x4ff445 in fuzzer::Fuzzer::Loop(std::__Fuzzer::vector >&) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:895:5 #11 0x4ee7af in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:912:6 #12 0x4ef078 in LLVMFuzzerRunDriver /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:925:10 #13 0x4deb35 in main (/build/fuzz-catalog+0x4deb35) #14 0x7f55e3a32082 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x24082) (BuildId: 1878e6b475720c7c51969e69ab2d276fae6d1dee) DEDUP_TOKEN: __interceptor_malloc--malloc--memdup SUMMARY: AddressSanitizer: double-free /src/llvm-project/compiler-rt/lib/asan/asan_malloc_linux.cpp:52:3 in __interceptor_free Found by Nallocfuzz. --- src/libsystemd/sd-journal/catalog.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/libsystemd/sd-journal/catalog.c b/src/libsystemd/sd-journal/catalog.c index 7527abf636..78f7f226c8 100644 --- a/src/libsystemd/sd-journal/catalog.c +++ b/src/libsystemd/sd-journal/catalog.c @@ -145,7 +145,8 @@ static int finish_item( char *payload, size_t payload_size) { _cleanup_free_ CatalogItem *i = NULL; - _cleanup_free_ char *prev = NULL, *combined = NULL; + _cleanup_free_ char *combined = NULL; + char *prev; assert(h); assert(payload); @@ -171,6 +172,7 @@ static int finish_item( if (ordered_hashmap_update(h, i, combined) < 0) return log_oom(); combined = NULL; + free(prev); } else { /* A new item */ combined = memdup(payload, payload_size + 1); From f32e44e4677d7f4e7ae83942b5d4c02fb349349d Mon Sep 17 00:00:00 2001 From: Frantisek Sumsal Date: Tue, 23 May 2023 19:21:20 +0200 Subject: [PATCH 2/7] sd-journal: use TAKE_PTR() a bit more --- src/libsystemd/sd-journal/catalog.c | 8 +++++--- src/resolve/resolved-dns-rr.c | 3 +-- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/src/libsystemd/sd-journal/catalog.c b/src/libsystemd/sd-journal/catalog.c index 78f7f226c8..7f1dc0b4ed 100644 --- a/src/libsystemd/sd-journal/catalog.c +++ b/src/libsystemd/sd-journal/catalog.c @@ -171,7 +171,8 @@ static int finish_item( if (ordered_hashmap_update(h, i, combined) < 0) return log_oom(); - combined = NULL; + + TAKE_PTR(combined); free(prev); } else { /* A new item */ @@ -181,8 +182,9 @@ static int finish_item( if (ordered_hashmap_put(h, i, combined) < 0) return log_oom(); - i = NULL; - combined = NULL; + + TAKE_PTR(i); + TAKE_PTR(combined); } return 0; diff --git a/src/resolve/resolved-dns-rr.c b/src/resolve/resolved-dns-rr.c index d3175b1b9d..78739d588d 100644 --- a/src/resolve/resolved-dns-rr.c +++ b/src/resolve/resolved-dns-rr.c @@ -1229,12 +1229,11 @@ int dns_resource_record_to_wire_format(DnsResourceRecord *rr, bool canonical) { assert(packet._data); free(rr->wire_format); - rr->wire_format = packet._data; + rr->wire_format = TAKE_PTR(packet._data); rr->wire_format_size = packet.size; rr->wire_format_rdata_offset = rds; rr->wire_format_canonical = canonical; - packet._data = NULL; dns_packet_unref(&packet); return 0; From b453ebf1c15935f1ba38fa6775ee26f223e29171 Mon Sep 17 00:00:00 2001 From: Frantisek Sumsal Date: Tue, 23 May 2023 21:34:48 +0200 Subject: [PATCH 3/7] resolve: avoid memory leak from a partially processed RR ==5==ERROR: LeakSanitizer: detected memory leaks Direct leak of 4096 byte(s) in 1 object(s) allocated from: #0 0x4a2056 in __interceptor_malloc /src/llvm-project/compiler-rt/lib/asan/asan_malloc_linux.cpp:69:3 #1 0x5180a9 in malloc (/build/fuzz-resource-record+0x5180a9) #2 0x4f7182 in dns_packet_extend /work/build/../../src/systemd/src/resolve/resolved-dns-packet.c:371:36 #3 0x4f8b8b in dns_packet_append_uint8 /work/build/../../src/systemd/src/resolve/resolved-dns-packet.c:433:13 #4 0x4f8b8b in dns_packet_append_name /work/build/../../src/systemd/src/resolve/resolved-dns-packet.c:597:13 #5 0x4f8f16 in dns_packet_append_key /work/build/../../src/systemd/src/resolve/resolved-dns-packet.c:622:13 #6 0x4fa9a0 in dns_packet_append_rr /work/build/../../src/systemd/src/resolve/resolved-dns-packet.c:883:13 #7 0x4eb00c in dns_resource_record_to_wire_format /work/build/../../src/systemd/src/resolve/resolved-dns-rr.c:1224:13 #8 0x4df7be in LLVMFuzzerTestOneInput /work/build/../../src/systemd/src/resolve/fuzz-resource-record.c:32:16 #9 0x518428 in NaloFuzzerTestOneInput (/build/fuzz-resource-record+0x518428) #10 0x537433 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:611:15 #11 0x536c1a 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 #12 0x5382e9 in fuzzer::Fuzzer::MutateAndTestOne() /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:757:19 #13 0x538fb5 in fuzzer::Fuzzer::Loop(std::__Fuzzer::vector >&) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:895:5 #14 0x52831f in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:912:6 #15 0x528be8 in LLVMFuzzerRunDriver /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:925:10 #16 0x5186a5 in main (/build/fuzz-resource-record+0x5186a5) #17 0x7f991fab8082 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x24082) (BuildId: 1878e6b475720c7c51969e69ab2d276fae6d1dee) DEDUP_TOKEN: __interceptor_malloc--malloc--dns_packet_extend SUMMARY: AddressSanitizer: 4096 byte(s) leaked in 1 allocation(s). Found by Nallocfuzz. --- src/resolve/resolved-dns-rr.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/resolve/resolved-dns-rr.c b/src/resolve/resolved-dns-rr.c index 78739d588d..44d1d1f1e6 100644 --- a/src/resolve/resolved-dns-rr.c +++ b/src/resolve/resolved-dns-rr.c @@ -1222,8 +1222,10 @@ int dns_resource_record_to_wire_format(DnsResourceRecord *rr, bool canonical) { return 0; r = dns_packet_append_rr(&packet, rr, 0, &start, &rds); - if (r < 0) + if (r < 0) { + dns_packet_unref(&packet); return r; + } assert(start == 0); assert(packet._data); From 6c13a39ac731a23c38685aa65b38bc0b10449b81 Mon Sep 17 00:00:00 2001 From: Frantisek Sumsal Date: Wed, 24 May 2023 11:39:24 +0200 Subject: [PATCH 4/7] specifier: avoid leaking memory on allocation error ==8036==ERROR: LeakSanitizer: detected memory leaks Direct leak of 64 byte(s) in 1 object(s) allocated from: #0 0x4a10bc in __interceptor_realloc /src/llvm-project/compiler-rt/lib/asan/asan_malloc_linux.cpp:85:3 #1 0x4deef1 in realloc (/build/fuzz-unit-file+0x4deef1) #2 0x7ffa35abfe23 in greedy_realloc /work/build/../../src/systemd/src/basic/alloc-util.c:70:13 #3 0x7ffa35aefad2 in parse_env_file_internal /work/build/../../src/systemd/src/basic/env-file.c:127:38 #4 0x7ffa35af08a6 in parse_env_file_fdv /work/build/../../src/systemd/src/basic/env-file.c:374:13 #5 0x7ffa35b6391e in parse_extension_release_atv /work/build/../../src/systemd/src/basic/os-util.c:323:16 #6 0x7ffa35b63c8a in parse_extension_release_sentinel /work/build/../../src/systemd/src/basic/os-util.c:360:13 #7 0x7ffa35a5e3f5 in parse_os_release_specifier /work/build/../../src/systemd/src/shared/specifier.c:292:13 #8 0x7ffa35a5e3f5 in specifier_os_id /work/build/../../src/systemd/src/shared/specifier.c:303:16 #9 0x7ffa35a5c7f5 in specifier_printf /work/build/../../src/systemd/src/shared/specifier.c:70:45 #10 0x7ffa3690b279 in unit_full_printf_full /work/build/../../src/systemd/src/core/unit-printf.c:264:16 #11 0x7ffa367de795 in config_parse_bus_name /work/build/../../src/systemd/src/core/load-fragment.c:2401:13 #12 0x7ffa358fe5ec in next_assignment /work/build/../../src/systemd/src/shared/conf-parser.c:151:24 #13 0x7ffa358fe5ec in parse_line /work/build/../../src/systemd/src/shared/conf-parser.c:257:16 #14 0x7ffa358fd653 in config_parse /work/build/../../src/systemd/src/shared/conf-parser.c:400:21 #15 0x4de828 in LLVMFuzzerTestOneInput /work/build/../../src/systemd/src/core/fuzz-unit-file.c:72:16 #16 0x4df208 in NaloFuzzerTestOneInput (/build/fuzz-unit-file+0x4df208) #17 0x4fe213 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:611:15 #18 0x4fd9fa 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 #19 0x4ff0c9 in fuzzer::Fuzzer::MutateAndTestOne() /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:757:19 #20 0x4ffd95 in fuzzer::Fuzzer::Loop(std::__Fuzzer::vector >&) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:895:5 #21 0x4ef0ff in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:912:6 #22 0x4ef9c8 in LLVMFuzzerRunDriver /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:925:10 #23 0x4df485 in main (/build/fuzz-unit-file+0x4df485) #24 0x7ffa35232082 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x24082) (BuildId: 1878e6b475720c7c51969e69ab2d276fae6d1dee) DEDUP_TOKEN: __interceptor_realloc--realloc--greedy_realloc SUMMARY: AddressSanitizer: 64 byte(s) leaked in 1 allocation(s). Found by Nallocfuzz. --- src/shared/specifier.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/shared/specifier.c b/src/shared/specifier.c index 31390fbd89..e5a1f94f2a 100644 --- a/src/shared/specifier.c +++ b/src/shared/specifier.c @@ -284,7 +284,7 @@ int specifier_architecture(char specifier, const void *data, const char *root, c * installation. */ static int parse_os_release_specifier(const char *root, const char *id, char **ret) { - char *v = NULL; + _cleanup_free_ char *v = NULL; int r; assert(ret); @@ -293,7 +293,7 @@ static int parse_os_release_specifier(const char *root, const char *id, char **r if (r >= 0) /* parse_os_release() calls parse_env_file() which only sets the return value for * entries found. Let's make sure we set the return value in all cases. */ - *ret = v; + *ret = TAKE_PTR(v); /* Translate error for missing os-release file to EUNATCH. */ return r == -ENOENT ? -EUNATCH : r; From 1469386b13531ead236013bfd23ae518dee603eb Mon Sep 17 00:00:00 2001 From: Frantisek Sumsal Date: Wed, 24 May 2023 11:41:30 +0200 Subject: [PATCH 5/7] env-file: use free_and_replace() --- src/basic/env-file.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/basic/env-file.c b/src/basic/env-file.c index 7b3e209ddc..58d7b3ec35 100644 --- a/src/basic/env-file.c +++ b/src/basic/env-file.c @@ -330,8 +330,7 @@ static int parse_env_file_push( if (streq(key, k)) { va_end(aq); - free(*v); - *v = value; + free_and_replace(*v, value); return 1; } From 08a8fd6e8de82a664762e7dd16df47227f75e2be Mon Sep 17 00:00:00 2001 From: Frantisek Sumsal Date: Wed, 24 May 2023 14:17:25 +0200 Subject: [PATCH 6/7] sd-journal: propagate errors from ordered_hashmap_*() Instead of masking them with -ENOMEM. --- src/libsystemd/sd-journal/catalog.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/libsystemd/sd-journal/catalog.c b/src/libsystemd/sd-journal/catalog.c index 7f1dc0b4ed..06cf396b95 100644 --- a/src/libsystemd/sd-journal/catalog.c +++ b/src/libsystemd/sd-journal/catalog.c @@ -147,6 +147,7 @@ static int finish_item( _cleanup_free_ CatalogItem *i = NULL; _cleanup_free_ char *combined = NULL; char *prev; + int r; assert(h); assert(payload); @@ -169,8 +170,9 @@ static int finish_item( if (!combined) return log_oom(); - if (ordered_hashmap_update(h, i, combined) < 0) - return log_oom(); + r = ordered_hashmap_update(h, i, combined); + if (r < 0) + return r; TAKE_PTR(combined); free(prev); @@ -180,8 +182,9 @@ static int finish_item( if (!combined) return log_oom(); - if (ordered_hashmap_put(h, i, combined) < 0) - return log_oom(); + r = ordered_hashmap_put(h, i, combined); + if (r < 0) + return r; TAKE_PTR(i); TAKE_PTR(combined); From f392dfb5a1286184189233a84f6d6871bd4f7ade Mon Sep 17 00:00:00 2001 From: Frantisek Sumsal Date: Wed, 24 May 2023 13:29:52 +0200 Subject: [PATCH 7/7] tree-wide: check memstream buffer after closing the handle When closing the FILE handle attached to a memstream, it may attempt to do a realloc() that may fail during OOM situations, in which case we are left with the buffer pointer pointing to NULL and buffer size > 0. For example: ``` #include #include #include void *realloc(void *ptr, size_t size) { return NULL; } int main(int argc, char *argv[]) { FILE *f; char *buf; size_t sz = 0; f = open_memstream(&buf, &sz); if (!f) return -ENOMEM; fputs("Hello", f); fflush(f); printf("buf: 0x%lx, sz: %lu, errno: %d\n", (unsigned long) buf, sz, errno); fclose(f); printf("buf: 0x%lx, sz: %lu, errno: %d\n", (unsigned long) buf, sz, errno); return 0; } ``` ``` $ gcc -o main main.c $ ./main buf: 0x74d4a0, sz: 5, errno: 0 buf: 0x0, sz: 5, errno: 0 ``` This might do unexpected things if the underlying code expects a valid pointer to the memstream buffer after closing the handle. Found by Nallocfuzz. --- src/basic/string-util.c | 17 ++--- src/busctl/busctl.c | 3 + src/core/manager-dump.c | 3 + src/coredump/coredump.c | 3 + src/journal/journalctl.c | 4 ++ src/libsystemd/sd-bus/bus-introspect.c | 4 ++ src/libsystemd/sd-bus/bus-match.c | 7 ++- src/network/generator/network-generator.c | 75 ++++++++++++----------- src/oom/oomd-manager.c | 3 + src/oom/oomd-util.c | 5 ++ src/resolve/resolved-dns-dnssec.c | 6 +- src/resolve/resolved-manager.c | 5 ++ src/shared/bus-util.c | 3 + src/shared/calendarspec.c | 18 +++--- src/shared/elf-util.c | 10 +++ src/shared/format-table.c | 3 + src/shared/json.c | 33 +++++----- 17 files changed, 132 insertions(+), 70 deletions(-) diff --git a/src/basic/string-util.c b/src/basic/string-util.c index c74ee67dfe..4d02ad5193 100644 --- a/src/basic/string-util.c +++ b/src/basic/string-util.c @@ -9,6 +9,7 @@ #include "alloc-util.h" #include "escape.h" #include "extract-word.h" +#include "fd-util.h" #include "fileio.h" #include "gunicode.h" #include "locale-util.h" @@ -603,9 +604,9 @@ char *strip_tab_ansi(char **ibuf, size_t *_isz, size_t highlight[2]) { STATE_CSI, STATE_CSO, } state = STATE_OTHER; - char *obuf = NULL; + _cleanup_free_ char *obuf = NULL; + _cleanup_fclose_ FILE *f = NULL; size_t osz = 0, isz, shift[2] = {}, n_carriage_returns = 0; - FILE *f; assert(ibuf); assert(*ibuf); @@ -713,11 +714,13 @@ char *strip_tab_ansi(char **ibuf, size_t *_isz, size_t highlight[2]) { } } - if (fflush_and_check(f) < 0) { - fclose(f); - return mfree(obuf); - } - fclose(f); + if (fflush_and_check(f) < 0) + return NULL; + + f = safe_fclose(f); + + if (!obuf) + return NULL; free_and_replace(*ibuf, obuf); diff --git a/src/busctl/busctl.c b/src/busctl/busctl.c index 965ded9675..4d69aee5eb 100644 --- a/src/busctl/busctl.c +++ b/src/busctl/busctl.c @@ -1072,6 +1072,9 @@ static int introspect(int argc, char **argv, void *userdata) { mf = safe_fclose(mf); + if (!buf) + return bus_log_parse_error(ENOMEM); + z = set_get(members, &((Member) { .type = "property", .interface = m->interface, diff --git a/src/core/manager-dump.c b/src/core/manager-dump.c index 5a92356d48..35143ebddf 100644 --- a/src/core/manager-dump.c +++ b/src/core/manager-dump.c @@ -95,6 +95,9 @@ int manager_get_dump_string(Manager *m, char **patterns, char **ret) { f = safe_fclose(f); + if (!dump) + return -ENOMEM; + *ret = TAKE_PTR(dump); return 0; diff --git a/src/coredump/coredump.c b/src/coredump/coredump.c index 5fdcfa7437..a6b0d96488 100644 --- a/src/coredump/coredump.c +++ b/src/coredump/coredump.c @@ -707,6 +707,9 @@ static int compose_open_fds(pid_t pid, char **open_fds) { if (errno > 0) return -errno; + if (!buffer) + return -ENOMEM; + *open_fds = TAKE_PTR(buffer); return 0; diff --git a/src/journal/journalctl.c b/src/journal/journalctl.c index 5f3a121ac9..e379203d5d 100644 --- a/src/journal/journalctl.c +++ b/src/journal/journalctl.c @@ -1811,6 +1811,10 @@ static int format_journal_url( return r; f = safe_fclose(f); + + if (!url) + return -ENOMEM; + *ret_url = TAKE_PTR(url); return 0; } diff --git a/src/libsystemd/sd-bus/bus-introspect.c b/src/libsystemd/sd-bus/bus-introspect.c index 49236a8e12..3d4c47c6dd 100644 --- a/src/libsystemd/sd-bus/bus-introspect.c +++ b/src/libsystemd/sd-bus/bus-introspect.c @@ -268,6 +268,10 @@ int introspect_finish(struct introspect *i, char **ret) { return r; i->f = safe_fclose(i->f); + + if (!i->introspection) + return -ENOMEM; + *ret = TAKE_PTR(i->introspection); return 0; diff --git a/src/libsystemd/sd-bus/bus-match.c b/src/libsystemd/sd-bus/bus-match.c index 703b9ac038..cc043abfe3 100644 --- a/src/libsystemd/sd-bus/bus-match.c +++ b/src/libsystemd/sd-bus/bus-match.c @@ -824,6 +824,7 @@ int bus_match_parse( char *bus_match_to_string(struct bus_match_component *components, size_t n_components) { _cleanup_free_ char *buffer = NULL; + _cleanup_fclose_ FILE *f = NULL; size_t size = 0; int r; @@ -832,7 +833,7 @@ char *bus_match_to_string(struct bus_match_component *components, size_t n_compo assert(components); - FILE *f = open_memstream_unlocked(&buffer, &size); + f = open_memstream_unlocked(&buffer, &size); if (!f) return NULL; @@ -855,9 +856,11 @@ char *bus_match_to_string(struct bus_match_component *components, size_t n_compo } r = fflush_and_check(f); - safe_fclose(f); if (r < 0) return NULL; + + f = safe_fclose(f); + return TAKE_PTR(buffer); } diff --git a/src/network/generator/network-generator.c b/src/network/generator/network-generator.c index 1090934bfc..569dcdf511 100644 --- a/src/network/generator/network-generator.c +++ b/src/network/generator/network-generator.c @@ -1199,30 +1199,31 @@ void link_dump(Link *link, FILE *f) { int network_format(Network *network, char **ret) { _cleanup_free_ char *s = NULL; + _cleanup_fclose_ FILE *f = NULL; size_t sz = 0; int r; assert(network); assert(ret); - { - _cleanup_fclose_ FILE *f = NULL; + f = open_memstream_unlocked(&s, &sz); + if (!f) + return -ENOMEM; - f = open_memstream_unlocked(&s, &sz); - if (!f) - return -ENOMEM; + network_dump(network, f); - network_dump(network, f); + /* Add terminating 0, so that the output buffer is a valid string. */ + fputc('\0', f); - /* Add terminating 0, so that the output buffer is a valid string. */ - fputc('\0', f); - - r = fflush_and_check(f); - } + r = fflush_and_check(f); if (r < 0) return r; - assert(s); + f = safe_fclose(f); + + if (!s) + return -ENOMEM; + *ret = TAKE_PTR(s); assert(sz > 0); return (int) sz - 1; @@ -1230,30 +1231,31 @@ int network_format(Network *network, char **ret) { int netdev_format(NetDev *netdev, char **ret) { _cleanup_free_ char *s = NULL; + _cleanup_fclose_ FILE *f = NULL; size_t sz = 0; int r; assert(netdev); assert(ret); - { - _cleanup_fclose_ FILE *f = NULL; + f = open_memstream_unlocked(&s, &sz); + if (!f) + return -ENOMEM; - f = open_memstream_unlocked(&s, &sz); - if (!f) - return -ENOMEM; + netdev_dump(netdev, f); - netdev_dump(netdev, f); + /* Add terminating 0, so that the output buffer is a valid string. */ + fputc('\0', f); - /* Add terminating 0, so that the output buffer is a valid string. */ - fputc('\0', f); - - r = fflush_and_check(f); - } + r = fflush_and_check(f); if (r < 0) return r; - assert(s); + f = safe_fclose(f); + + if (!s) + return -ENOMEM; + *ret = TAKE_PTR(s); assert(sz > 0); return (int) sz - 1; @@ -1261,30 +1263,31 @@ int netdev_format(NetDev *netdev, char **ret) { int link_format(Link *link, char **ret) { _cleanup_free_ char *s = NULL; + _cleanup_fclose_ FILE *f = NULL; size_t sz = 0; int r; assert(link); assert(ret); - { - _cleanup_fclose_ FILE *f = NULL; + f = open_memstream_unlocked(&s, &sz); + if (!f) + return -ENOMEM; - f = open_memstream_unlocked(&s, &sz); - if (!f) - return -ENOMEM; + link_dump(link, f); - link_dump(link, f); + /* Add terminating 0, so that the output buffer is a valid string. */ + fputc('\0', f); - /* Add terminating 0, so that the output buffer is a valid string. */ - fputc('\0', f); - - r = fflush_and_check(f); - } + r = fflush_and_check(f); if (r < 0) return r; - assert(s); + f = safe_fclose(f); + + if (!s) + return -ENOMEM; + *ret = TAKE_PTR(s); assert(sz > 0); return (int) sz - 1; diff --git a/src/oom/oomd-manager.c b/src/oom/oomd-manager.c index 08a29ec77b..3f050cdbb2 100644 --- a/src/oom/oomd-manager.c +++ b/src/oom/oomd-manager.c @@ -847,6 +847,9 @@ int manager_get_dump_string(Manager *m, char **ret) { f = safe_fclose(f); + if (!dump) + return -ENOMEM; + *ret = TAKE_PTR(dump); return 0; } diff --git a/src/oom/oomd-util.c b/src/oom/oomd-util.c index 49c10b5e16..6309d2c473 100644 --- a/src/oom/oomd-util.c +++ b/src/oom/oomd-util.c @@ -299,6 +299,11 @@ static int dump_kill_candidates(OomdCGroupContext **sorted, int n, int dump_unti if (r < 0) return r; + f = safe_fclose(f); + + if (!dump) + return -ENOMEM; + return log_dump(LOG_INFO, dump); } diff --git a/src/resolve/resolved-dns-dnssec.c b/src/resolve/resolved-dns-dnssec.c index fc076856b6..e7c18c29a0 100644 --- a/src/resolve/resolved-dns-dnssec.c +++ b/src/resolve/resolved-dns-dnssec.c @@ -867,10 +867,14 @@ static int dnssec_rrset_serialize_sig( } r = fflush_and_check(f); - f = safe_fclose(f); /* sig_data may be reallocated when f is closed. */ if (r < 0) return r; + f = safe_fclose(f); /* sig_data may be reallocated when f is closed. */ + + if (!sig_data) + return -ENOMEM; + *ret_sig_data = TAKE_PTR(sig_data); *ret_sig_size = sig_size; return 0; diff --git a/src/resolve/resolved-manager.c b/src/resolve/resolved-manager.c index 184d8e3f3d..67786d39fd 100644 --- a/src/resolve/resolved-manager.c +++ b/src/resolve/resolved-manager.c @@ -519,6 +519,11 @@ static int manager_sigusr1(sd_event_source *s, const struct signalfd_siginfo *si if (fflush_and_check(f) < 0) return log_oom(); + f = safe_fclose(f); + + if (!buffer) + return -ENOMEM; + log_dump(LOG_INFO, buffer); return 0; } diff --git a/src/shared/bus-util.c b/src/shared/bus-util.c index ca0d77c136..b99883a088 100644 --- a/src/shared/bus-util.c +++ b/src/shared/bus-util.c @@ -628,6 +628,9 @@ static int method_dump_memory_state_by_fd(sd_bus_message *message, void *userdat dump_file = safe_fclose(dump_file); + if (!dump) + return -ENOMEM; + fd = acquire_data_fd(dump, dump_size, 0); if (fd < 0) return fd; diff --git a/src/shared/calendarspec.c b/src/shared/calendarspec.c index 86a6d3f608..5690e3144a 100644 --- a/src/shared/calendarspec.c +++ b/src/shared/calendarspec.c @@ -12,6 +12,7 @@ #include "alloc-util.h" #include "calendarspec.h" #include "errno-util.h" +#include "fd-util.h" #include "fileio.h" #include "macro.h" #include "parse-util.h" @@ -337,9 +338,9 @@ static void format_chain(FILE *f, int space, const CalendarComponent *c, bool us } int calendar_spec_to_string(const CalendarSpec *c, char **p) { - char *buf = NULL; + _cleanup_free_ char *buf = NULL; + _cleanup_fclose_ FILE *f = NULL; size_t sz = 0; - FILE *f; int r; assert(c); @@ -384,14 +385,15 @@ int calendar_spec_to_string(const CalendarSpec *c, char **p) { } r = fflush_and_check(f); - fclose(f); - - if (r < 0) { - free(buf); + if (r < 0) return r; - } - *p = buf; + f = safe_fclose(f); + + if (!buf) + return -ENOMEM; + + *p = TAKE_PTR(buf); return 0; } diff --git a/src/shared/elf-util.c b/src/shared/elf-util.c index 98c47d9125..5885215a1c 100644 --- a/src/shared/elf-util.c +++ b/src/shared/elf-util.c @@ -621,8 +621,13 @@ static int parse_core(int fd, const char *executable, char **ret, JsonVariant ** return log_warning_errno(r, "Could not parse core file, flushing file buffer failed: %m"); c.f = safe_fclose(c.f); + + if (!buf) + return log_oom(); + *ret = TAKE_PTR(buf); } + if (ret_package_metadata) *ret_package_metadata = TAKE_PTR(package_metadata); @@ -735,8 +740,13 @@ static int parse_elf(int fd, const char *executable, char **ret, JsonVariant **r return log_warning_errno(r, "Could not parse ELF file, flushing file buffer failed: %m"); c.f = safe_fclose(c.f); + + if (!buf) + return log_oom(); + *ret = TAKE_PTR(buf); } + if (ret_package_metadata) *ret_package_metadata = TAKE_PTR(elf_metadata); diff --git a/src/shared/format-table.c b/src/shared/format-table.c index 204e8b68b6..83b749d677 100644 --- a/src/shared/format-table.c +++ b/src/shared/format-table.c @@ -2537,6 +2537,9 @@ int table_format(Table *t, char **ret) { f = safe_fclose(f); + if (!buf) + return -ENOMEM; + *ret = TAKE_PTR(buf); return 0; diff --git a/src/shared/json.c b/src/shared/json.c index 73050b55c8..5bf00f009f 100644 --- a/src/shared/json.c +++ b/src/shared/json.c @@ -1769,6 +1769,7 @@ static int json_format(FILE *f, JsonVariant *v, JsonFormatFlags flags, const cha int json_variant_format(JsonVariant *v, JsonFormatFlags flags, char **ret) { _cleanup_free_ char *s = NULL; + _cleanup_fclose_ FILE *f = NULL; size_t sz = 0; int r; @@ -1781,26 +1782,26 @@ int json_variant_format(JsonVariant *v, JsonFormatFlags flags, char **ret) { if (flags & JSON_FORMAT_OFF) return -ENOEXEC; - { - _cleanup_fclose_ FILE *f = NULL; + f = open_memstream_unlocked(&s, &sz); + if (!f) + return -ENOMEM; - f = open_memstream_unlocked(&s, &sz); - if (!f) - return -ENOMEM; - - r = json_variant_dump(v, flags, f, NULL); - if (r < 0) - return r; - - /* Add terminating 0, so that the output buffer is a valid string. */ - fputc('\0', f); - - r = fflush_and_check(f); - } + r = json_variant_dump(v, flags, f, NULL); if (r < 0) return r; - assert(s); + /* Add terminating 0, so that the output buffer is a valid string. */ + fputc('\0', f); + + r = fflush_and_check(f); + if (r < 0) + return r; + + f = safe_fclose(f); + + if (!s) + return -ENOMEM; + *ret = TAKE_PTR(s); assert(sz > 0); return (int) sz - 1;