diff options
author | Zbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl> | 2021-03-04 21:19:02 +0100 |
---|---|---|
committer | Zbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl> | 2021-03-12 18:22:15 +0100 |
commit | 431a7d069b55866b4802a29eb5d6e8ccb5f6b9fd (patch) | |
tree | 0594acabacc5704e954c56a794f5c660e396feeb | |
parent | 29130b5d51d46e1bf2380d288ccc9290dc4a419d (diff) | |
download | systemd-431a7d069b55866b4802a29eb5d6e8ccb5f6b9fd.tar.gz |
sd-bus: fix memstream buffer extraction
I'm getting the following error under valgrind:
==305970== Invalid free() / delete / delete[] / realloc()
==305970== at 0x483E9F1: free (vg_replace_malloc.c:538)
==305970== by 0x4012CD: mfree (alloc-util.h:48)
==305970== by 0x4012EF: freep (alloc-util.h:83)
==305970== by 0x4017F4: LLVMFuzzerTestOneInput (fuzz-bus-match.c:58)
==305970== by 0x401A58: main (fuzz-main.c:39)
==305970== Address 0x59972f0 is 0 bytes inside a block of size 8,192 free'd
==305970== at 0x483FCE4: realloc (vg_replace_malloc.c:834)
==305970== by 0x4C986F7: _IO_mem_finish (in /usr/lib64/libc-2.33.so)
==305970== by 0x4C8F5E0: fclose@@GLIBC_2.2.5 (in /usr/lib64/libc-2.33.so)
==305970== by 0x49D2CDB: fclose_nointr (fd-util.c:108)
==305970== by 0x49D2D3D: safe_fclose (fd-util.c:124)
==305970== by 0x4A4BCCC: fclosep (fd-util.h:41)
==305970== by 0x4A4E00F: bus_match_to_string (bus-match.c:859)
==305970== by 0x4016C2: LLVMFuzzerTestOneInput (fuzz-bus-match.c:58)
==305970== by 0x401A58: main (fuzz-main.c:39)
==305970== Block was alloc'd at
==305970== at 0x483FAE5: calloc (vg_replace_malloc.c:760)
==305970== by 0x4C98787: open_memstream (in /usr/lib64/libc-2.33.so)
==305970== by 0x49D56D6: open_memstream_unlocked (fileio.c:97)
==305970== by 0x4A4DEC5: bus_match_to_string (bus-match.c:859)
==305970== by 0x4016C2: LLVMFuzzerTestOneInput (fuzz-bus-match.c:58)
==305970== by 0x401A58: main (fuzz-main.c:39)
==305970==
So the fclose() which is called from _cleanup_fclose_ clearly reallocates the
buffer (maybe to save memory?). open_memstream(3) says:
The locations referred to by these pointers are updated each time the
stream is flushed (fflush(3)) and when the stream is closed (fclose(3)).
This seems to mean that we should close the stream first before grabbing the
buffer pointer.
(cherry picked from commit 5963e6f43c4f33d5255ef0fb887cdf382bd51c9e)
(cherry picked from commit f8fd75183bcf9cd6b55c3f8e752863d0083ed772)
-rw-r--r-- | src/libsystemd/sd-bus/bus-match.c | 9 |
1 files changed, 4 insertions, 5 deletions
diff --git a/src/libsystemd/sd-bus/bus-match.c b/src/libsystemd/sd-bus/bus-match.c index 57ce8cca04..9066f68e35 100644 --- a/src/libsystemd/sd-bus/bus-match.c +++ b/src/libsystemd/sd-bus/bus-match.c @@ -856,8 +856,7 @@ fail: } char *bus_match_to_string(struct bus_match_component *components, unsigned n_components) { - _cleanup_fclose_ FILE *f = NULL; - char *buffer = NULL; + _cleanup_free_ char *buffer = NULL; size_t size = 0; unsigned i; int r; @@ -867,7 +866,7 @@ char *bus_match_to_string(struct bus_match_component *components, unsigned n_com assert(components); - f = open_memstream_unlocked(&buffer, &size); + FILE *f = open_memstream_unlocked(&buffer, &size); if (!f) return NULL; @@ -890,10 +889,10 @@ char *bus_match_to_string(struct bus_match_component *components, unsigned n_com } r = fflush_and_check(f); + safe_fclose(f); if (r < 0) return NULL; - - return buffer; + return TAKE_PTR(buffer); } int bus_match_add( |