basic/random-util: do not use getrandom() under msan
`fuzz-journal-remote` seems to be failing under `msan` as soon as it starts:
$ sudo infra/helper.py run_fuzzer systemd fuzz-journal-remote
Running: docker run --rm -i --privileged -e FUZZING_ENGINE=libfuzzer -v /home/vagrant/oss-fuzz/build/out/systemd:/out -t gcr.io/oss-fuzz-base/base-runner run_fuzzer fuzz-journal-remote
Using seed corpus: fuzz-journal-remote_seed_corpus.zip
/out/fuzz-journal-remote -rss_limit_mb=2048 -timeout=25 /tmp/fuzz-journal-remote_corpus -max_len=65536 < /dev/null
INFO: Seed: 3380449479
INFO: Loaded 2 modules (36336 inline 8-bit counters): 36139 [0x7ff36ea31d39, 0x7ff36ea3aa64), 197 [0x9998c8, 0x99998d),
INFO: Loaded 2 PC tables (36336 PCs): 36139 [0x7ff36ea3aa68,0x7ff36eac7d18), 197 [0x999990,0x99a5e0),
INFO: 2 files found in /tmp/fuzz-journal-remote_corpus
INFO: seed corpus: files: 2 min: 4657b max: 7790b total: 12447b rss: 97Mb
Uninitialized bytes in __interceptor_pwrite64 at offset 24 inside [0x7fffdd4d7230, 240)
==15==WARNING: MemorySanitizer: use-of-uninitialized-value
#0 0x7ff36e685e8a in journal_file_init_header /work/build/../../src/systemd/src/journal/journal-file.c:436:13
#1 0x7ff36e683a9d in journal_file_open /work/build/../../src/systemd/src/journal/journal-file.c:3333:21
#2 0x7ff36e68b8f6 in journal_file_open_reliably /work/build/../../src/systemd/src/journal/journal-file.c:3520:13
#3 0x4a3f35 in open_output /work/build/../../src/systemd/src/journal-remote/journal-remote.c:70:13
#4 0x4a34d0 in journal_remote_get_writer /work/build/../../src/systemd/src/journal-remote/journal-remote.c:136:21
#5 0x4a550f in get_source_for_fd /work/build/../../src/systemd/src/journal-remote/journal-remote.c:183:13
#6 0x4a46bd in journal_remote_add_source /work/build/../../src/systemd/src/journal-remote/journal-remote.c:235:13
#7 0x4a271c in LLVMFuzzerTestOneInput /work/build/../../src/systemd/src/fuzz/fuzz-journal-remote.c:36:9
#8 0x4f27cc in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) /src/libfuzzer/FuzzerLoop.cpp:524:13
#9 0x4efa0b in fuzzer::Fuzzer::RunOne(unsigned char const*, unsigned long, bool, fuzzer::InputInfo*, bool*) /src/libfuzzer/FuzzerLoop.cpp:448:3
#10 0x4f8e96 in fuzzer::Fuzzer::ReadAndExecuteSeedCorpora(std::__1::vector<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, fuzzer::fuzzer_allocator<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > > > const&) /src/libfuzzer/FuzzerLoop.cpp:732:7
#11 0x4f9f73 in fuzzer::Fuzzer::Loop(std::__1::vector<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, fuzzer::fuzzer_allocator<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > > > const&) /src/libfuzzer/FuzzerLoop.cpp:752:3
#12 0x4bf329 in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) /src/libfuzzer/FuzzerDriver.cpp:756:6
#13 0x4ac391 in main /src/libfuzzer/FuzzerMain.cpp:20:10
#14 0x7ff36d14982f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2082f)
#15 0x41f9d8 in _start (/out/fuzz-journal-remote+0x41f9d8)
Uninitialized value was stored to memory at
#0 0x7ff36e61cd41 in sd_id128_randomize /work/build/../../src/systemd/src/libsystemd/sd-id128/sd-id128.c:288:16
#1 0x7ff36e685cec in journal_file_init_header /work/build/../../src/systemd/src/journal/journal-file.c:426:13
#2 0x7ff36e683a9d in journal_file_open /work/build/../../src/systemd/src/journal/journal-file.c:3333:21
#3 0x7ff36e68b8f6 in journal_file_open_reliably /work/build/../../src/systemd/src/journal/journal-file.c:3520:13
#4 0x4a3f35 in open_output /work/build/../../src/systemd/src/journal-remote/journal-remote.c:70:13
#5 0x4a34d0 in journal_remote_get_writer /work/build/../../src/systemd/src/journal-remote/journal-remote.c:136:21
#6 0x4a550f in get_source_for_fd /work/build/../../src/systemd/src/journal-remote/journal-remote.c:183:13
#7 0x4a46bd in journal_remote_add_source /work/build/../../src/systemd/src/journal-remote/journal-remote.c:235:13
#8 0x4a271c in LLVMFuzzerTestOneInput /work/build/../../src/systemd/src/fuzz/fuzz-journal-remote.c:36:9
#9 0x4f27cc in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) /src/libfuzzer/FuzzerLoop.cpp:524:13
#10 0x4efa0b in fuzzer::Fuzzer::RunOne(unsigned char const*, unsigned long, bool, fuzzer::InputInfo*, bool*) /src/libfuzzer/FuzzerLoop.cpp:448:3
#11 0x4f8e96 in fuzzer::Fuzzer::ReadAndExecuteSeedCorpora(std::__1::vector<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, fuzzer::fuzzer_allocator<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > > > const&) /src/libfuzzer/FuzzerLoop.cpp:732:7
#12 0x4f9f73 in fuzzer::Fuzzer::Loop(std::__1::vector<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, fuzzer::fuzzer_allocator<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > > > const&) /src/libfuzzer/FuzzerLoop.cpp:752:3
#13 0x4bf329 in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) /src/libfuzzer/FuzzerDriver.cpp:756:6
#14 0x4ac391 in main /src/libfuzzer/FuzzerMain.cpp:20:10
#15 0x7ff36d14982f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2082f)
Uninitialized value was created by an allocation of 't' in the stack frame of function 'sd_id128_randomize'
#0 0x7ff36e61cb00 in sd_id128_randomize /work/build/../../src/systemd/src/libsystemd/sd-id128/sd-id128.c:274
SUMMARY: MemorySanitizer: use-of-uninitialized-value /work/build/../../src/systemd/src/journal/journal-file.c:436:13 in journal_file_init_header
Exiting
MS: 0 ; base unit: 0000000000000000000000000000000000000000
artifact_prefix='./'; Test unit written to ./crash-847911777b3096783f4ee70a69ab6d28380c810b
[vagrant@localhost oss-fuzz]$ sudo infra/helper.py check_build --sanitizer=memory systemd
Running: docker run --rm -i --privileged -e FUZZING_ENGINE=libfuzzer -e SANITIZER=memory -v /home/vagrant/oss-fuzz/build/out/systemd:/out -t gcr.io/oss-fuzz-base/base-runner test_all
INFO: performing bad build checks for /out/fuzz-dhcp-server.
INFO: performing bad build checks for /out/fuzz-journal-remote.
INFO: performing bad build checks for /out/fuzz-unit-file.
INFO: performing bad build checks for /out/fuzz-dns-packet.
4 fuzzers total, 0 seem to be broken (0%).
Check build passed.
It's a false positive which is most likely caused by
https://github.com/google/sanitizers/issues/852. I think it could be got around
by avoiding `getrandom` when the code is compiled with `msan`
basic/journal-importer: escape & ellipsize bad data in log entries
We shouldn't just log arbitrary stuff, in particular newlines and control chars
Now:
Unknown dunder line __CURSORFACILITY=6\nSYSLOG_IDENTIFIER=/USR/SBIN/CRON\nMES…, ignoring.
Unknown dunder line __REALTIME_TIME[TAMP=1404101101501874\n__MONOTONIC_TIMEST…, ignoring.
basic/string-util: add a convenience function to cescape mostly-ascii fields
It's not supposed to be the most efficient, but instead fast and simple to use.
I kept the logic in ellipsize_mem() to use unicode ellipsis even in non-unicode
locales. I'm not quite convinced things should be this way, especially that with
this patch it'd actually be simpler to always use "…" in unicode locale and "..."
otherwise, but Lennart wanted it this way for some reason.
journal-remote: when an entry is rejected with -EBADMSG, do not rotate the journal file
Something is wrong with the entry (probably a missing timestamp), so no point
in rotating. But suppress the error in process_source(), so that the processing
of the data stream continues.
Also, just return 0 from writer_write() on success, the only caller doesn't
care.
journal: allow writing journal files even if machine-id is missing
The code to open journal files seems like the wrong place to enforce this. We
already check during boot and refuse to boot if machine-id is missing, no need
to enforce this here. In particular, it seems better to write logs from
journald even if they are not completely functional rather than refuse to
operate at all, and systemd-journal-remote also writes journal files and may
even be run on a system without systemd at all.
The docker image that oss-fuzz uses has an empty /etc/machine-id. Obviously
this is an error in the docker, but docker is fact of life, and it seems better
for systemd-journal-remote to work in such an incomplete environment.
We currently return -ENOMEDIUM when /etc/machine-id is empty, and -EINVAL when
it is all zeros. But -EINVAL is also used for invalid args. The distinction
between empty and all-zero is not very important, let's use the same return
code.
Also document -ENOENT and -ENOMEDIUM since they can be a bit surprising.
journal-remote: split out µhttpd support and main() into a separate file
This is in preparation to reusing the RemoteServer in other concepts.
I tried to keep changes to minimum:
- arg_* global variables are now passed as state in RemoteServer
- exported functions get the "journal_remote_" prefix
- some variables are renamed
In particular, there is an ugly global RemoveServer* variable. It was originally
added because µhttpd did not allow state to be passed to the callbacks. I'm not
sure if this has been remediated in µhttpd, but either way, this is not changed
here, the global variable is only renamed for clarity.
smack: make mac_smack_fix() deal somewhat sensible with non-absolute paths
This tries to improve the mac_smack_fix() logic a bit, by properly
handling non-absolute paths.
It's still pretty broken though, which is sad for security technology:
non-normalized paths (for example "/usr/../dev/sda") will still not be
treated correctly. I am not sure how to fix that properly though, and I
don't understand SMACK well enough to do so. This fix hence just fixes
to most obvious glaring issue.
This introduces several macros for defining config parsers.
Also this fixes errno in DEFINE_CONFIG_PARSE_ENUM() and _ENUMV()
and makes the log level lower when a duplicated item is
specified to the settings parsed by the function defined by
DEFINE_CONFIG_PARSE_ENUMV().
sd-bus: make add match method callback slot "floating"
When we allocate an asynchronous match object we will allocate an
asynchronous bus call object to install the match server side.
Previously the call slot would be created as regular slot, i.e.
non-floating which meant installing the match even if it was itself
floating would result in a non-floating slot to be created internally,
which ultimately would mean the sd_bus object would be referenced by it,
and thus never be freed.
Let's fix that by making the match method callback floating in any case
as we have no interest in leaving the bus allocated beyond the match
slot.
This new call allows explicit control of the "floating" state of a bus
slot object. This is useful for creating a bus slot object first,
retaining a reference to it, using it for making changes to the slot
object (for example, set a description) and then handing it over to
sd-bus for lifecycle management.
Vito Caputo [Mon, 28 May 2018 22:09:55 +0000 (15:09 -0700)]
journal-file: avoid joining offline thread
In journal_file_set_online() the offline thread doesn't need to be
joined if it's been canceled before actually reaching the phase of
writing the offline state.
We usually seperate case statements within a switch from each other by
empty lines. We also often add an empty line after multi-line function
prototypes, let's do so here too
This corresponds nicely with the specifiers we already pass for
/var/lib, /var/cache, /run and so on.
This is particular useful to update the test-path service files to
operate without guessable files, thus allowing multiple parallel
test-path invocations to pass without issues (the idea is to set $TMPDIR
early on in the test to some private directory, and then only use the
new %T or %V specifier to refer to it).
man: sort specifier list in systemd.unit(5) alphabetically
Usually, we order our settings in our unit files in a logical order,
grouping related settings together, and putting more relevant stuff
first, instead of following a strictly alphabetical order.
For specifiers I think it makes sense to follow an alphabetical order
however, since they literally are just characters, and hence I think the
concept of alphabetical ordering is much more commanding for them. Also,
since specifiers are usually not used in combination, but mostly used
indepdently of each other I think it's not that important to group
similar ones together.
meson: use a convenience static library for nspawn core
This makes it easier to link the nspawn implementation to the tests.
Right now this just means that nspawn-patch-uid.c is not compiled
twice, which is nice, but results in test-patch-uid being slightly bigger,
which is not nice. But in general, we should use convenience libs to
compile everything just once, as far as possible. Otherwise, once we
start compiling a few files here twice, and a few file there thrice, we
soon end up in a state where we are doing hundreds of extra compilations.
So let's do the "right" thing, even if is might not be more efficient.