Also use structued initialization in one more place, use '\0' for NUL bytes,
and move variable to the right block (the code was OK, but it is strange to
have 'char *value' defined in a different scope then 'size_t value_allocated').
The fuzzer seems to have no trouble with this sample. It seems that the
problem reported in the bug is not caused by the match parsing code. But
let's add the sample just in case.
This fuzzer is based on test-bus-match. Even the initial corpus is
derived entirely from it.
https://bugzilla.redhat.com/show_bug.cgi?id=1935084 shows an crash
in bus_match_parse(). I checked the coverage stats on oss-fuzz, and
sadly existing fuzzing did not cover this code at all.
==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.
Michal Sekletar [Thu, 4 Mar 2021 16:35:22 +0000 (17:35 +0100)]
udev: run link_update() with increased retry count in second invocation
In PR #17431 we have introduced retry loop in link_update() in order to
maximize the chance that we end up with correct target when there are
multiple contenders for given symlink.
Number of iterations in retry loop is either 1 or
LINK_UPDATE_MAX_RETRIES, depending on the value of 'initialized' db
flag. When device appears for the first time we need to set the
flag before calling link_update() via update_devnode() for the second
time to make sure we run the second invocation with higher retry loop
counter.
When running integration tests under sanitizers D-Bus fails to
shutdown cleanly, causing unnecessary noise in the logs:
```
dbus-daemon[272]: ==272==LeakSanitizer has encountered a fatal error.
dbus-daemon[272]: ==272==HINT: For debugging, try setting environment variable LSAN_OPTIONS=verbosity=1:log_threads=1
dbus-daemon[272]: ==272==HINT: LeakSanitizer does not work under ptrace (strace, gdb, etc)
```
Since we're not "sanitizing" D-Bus anyway let's disable LSan's at_exit
check for the dbus.service to get rid of this error.
Luca Boccassi [Fri, 5 Mar 2021 20:19:44 +0000 (20:19 +0000)]
dissect: avoid overflow access by NULLSTR_FOREACH
NULLSTR_FOREACH expects two terminating NULs, but the joined string
for extension-release.d only had the canonical one.
Use a placeholder when joining and fix it manually.
sysctl-util: use read_full_virtual_file() for reading sysctls
Given these files are part of procfs, let's use the correct API calls
for reading them.
This changes one occasion of read_one_line_file() to
read_full_virtual_file(), which superficially is a different thing, but
shouldn't actually be a difference, since sysctls can't be longer than
4K anyway, and the piecemeal logic behind read_one_line_file() cannot
work with the special semantics of procfs anyway.
run: tweak algorithm for generating unit name from dbus unique name
This reverts behaviour of systemd-run's unit name generation to the
status quo ante of #18871: we chop off the ":1." prefix if we can.
However, to address the issue that the unique name can overrun we then
do what #18871 did as fallback: only chop off the ":" prefix.
This way we should have pretty names that look like they always looked
in the common case, but in the case of a unique name overrun we still
will have names that work.
rm-rf: fix up chmod in the _cleanup_ rm_rf() destructors
REMOVE_CHMOD is necessary to remove files/dirs that are owned by us but
have an access mode that would not allow us to remove them. In generic
destructor calls for use with `_cleanup_` that are "fire-and-forget"
style we should make use of that, to maximize the chance we can actually
remove the files/dirs.
(Also, add in REMOVE_MISSING_OK. Just because prettier, we ignore the
return codes anyway, but it' a bit nicer to ignore a bit fewer errors.)
localed: refuse to set a keymap which is not installed
In https://bugzilla.redhat.com/show_bug.cgi?id=1933873 a keymap was set without
the package that provides it being installed (it2 is in kbd-legacy, which is
not installed by default). Setting a non-installed keymap is problematic,
because it results in nasty failures afterward (*). So let's to the same as
e.g. for locale data, and refuse a setting if the definition doesn't exists in
the filesystem.
The implementation using nftw() is not the most efficient, but I think it's OK
in this case. This is definitely not in any kind of hot path, and I prefer not
to duplicate the filename manipulation logic in a second function.
(*) If the keymap is not found, vconsole-setup.service will fail.
dracut-cmdline-ask.service has Requires=vconsole-setup.service, so it will also
fail, and this breaks boot. dracut-cmdline-ask.service having a hard dependency
is appropriate though: we sadly don't display what the keymap is, and with a wrong
keymap, any attempts to enter a password are likely to fail.
shared/kbd-util: fix return value confusion with nftw()
We would return a real error sometimes from the callback, and FTW_STOP other
times. Because of FTW_ACTIONRETVAL, everything except FTW_STOP would be
ignored. I don't think using FTW_ACTIONRETVAL is useful.
nftw() can only be used meaningfully with errno. Even if we return a proper
value ourselves from the callback, it will be propagated as a return value from
nftw(), but there is no way to distinguish this from a value generated by
nftw() itself, which is -1/-EPERM on error. So let's set errno ourselves so the
caller can at least look at that.
Anita Zhang [Thu, 4 Mar 2021 01:25:40 +0000 (17:25 -0800)]
run: update dbus unique names check
Some code in systemd-run checks that a bus's unique name must start with
`:1.`. However the dbus specification on unique connection names only specifies
that it must begin with a colon. And the freedesktop/dbus implementation allows
allows unique names to go up to `:INT_MAX.INT_MAX`. So update the
current check to only look for a colon at the beginning.
Nominally, the bug was in unit_load_dropin(), which just took the last mtime
instead of calculating the maximum. But instead of adding code to wrap the
loop, this patch goes in the other direction.
All (correct) callers of config_parse() followed a very similar pattern to
calculate the maximum mtime. So let's simplify things by making config_parse()
assume that mtime is initialized and update it to the maximum. This makes all
the callers that care about mtime simpler and also fixes the issue in
unit_load_dropin().
config_parse_many_nulstr() and config_parse_many() are different, because it
makes sense to call them just once, and current ret_mtime behaviour make sense.
rules: Move ID_SMARTCARD_READER definition to a <70 configuration.
70-uaccess.rules sets the uaccess tag on devices with ID_SMARTCARD_READER
set, but it is set in 99-systemd.rules .
Move this to a 60-*.rules which already matches USB CCID class, factorising
the matching, so 70-uaccess.rules sets up these devices as expected.
It's useful to be able to combine a regular /usr/ file system with a
tmpfs as root, for an OS that boots up in volatile mode on every single
boot. Let's add explicit support for this via root=tmpfs.
Note the relationship to the existing systemd.volatile= option:
1. The kernel command line "root=/dev/… systemd.volatile=yes" will mount
the specified root fs, and then hide everything at the top by
overmounting it with a tmpfs, except for the /usr subtree.
2. The kernel command line "root=tmpfs mount.usr=/dev/…" otoh will mount
a toot fs at the top (just like the case above), but will then mount
the top-level dir of the fs specified in mount.usr= directly below
it.
Or to say this differently: in the first case /usr/ from the physical
storage fs is going to become /usr/ of the hierarchy ultimately booted,
while in the second case / from the physical storage fs is going to
become /usr of the hierarchy booted.
Philosophically I figure systemd.volatile= is more an option for
"one-off" boots, while root=tmpfs is something to have as default mode
of operation for suitable images.
This is currently hard to test reasonably, since Dracut refuses to
accept root=tmpfs. This needs to be addressed separately though.
path-util: return O_DIRECTORY from path_extract_filename() when path ends in slash
Let's fine-tune the path_extract_filename() interface: on succes return
O_DIRECTORY as indicator that the input path was slash-suffixed, and
regular 0 otherwise. This is useful since in many cases it is useful to
filter out paths that must refer to dirs early on.
I opted for O_DIRECTORY instead of the following other ideas:
1. return -EISDIR: I think the function should return an extracted
filename even when referring to an obvious dir, so this is not an
option.
2. S_ISDIR, this was a strong contender, but I think O_DIRECTORY is a
tiny bit nicer since quite likely we will go on and open the thing,
maybe with openat(), and hence it's quite nice to be able to OR in
the return value into the flags argument of openat().
3. A new enum defined with two values "dont-know" and
"definitely-directory". But I figured this was unnecessary, given we
have other options too, that reuse existing definitions for very
similar purposes.
path-util: add path_extract_directory(), to match path_extract_filename()
These two together are a lot like dirname() + basename() but have the
benefit that they return clear errors when one passes a special case
path to them where the extraction doesn't make sense, i.e. "", "/",
"foo", "foo/" and so on.
Sooner or later we should probably port all our uses of
dirname()/basename() over to this, to catch these special cases more
safely.
projects / thirdparty / systemd.git / log
