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.
Fixes: #8551
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.
It's also useful to fix#8551.
This adds a small service "systemd-portabled" and a matching client
"portablectl", which implement the "portable service" concept.
The daemon implements the actual operations, is PolicyKit-enabled and is
activated on demand with exit-on-idle.
Both the daemon and the client are an optional build artifact, enabled
by default rhough.
Most our other parsing functions do this, let's do this here too,
internally we accept that anyway. Also, the closely related
load_env_file() and load_env_file_pairs() also do this, so let's be
systematic.
Let's tweak the assignment of errors a bit, and automatically abs()
errnos, similar to how log_error_errno() and friends does it.
Macros are fine to use, but regular functions usually preferable if
there's no reason to use macros, because they avoid multiple evaluation
and suchlike. Hence, let's just use a regular funciton for assigning
errors, instead of macros.
Follow-up for #8993
The protocol is that a string is serialized with the nul byte at the end, and
the terminator is included in length. We'd call strndup with offset 0, length
len1-1, and then a second time with offset len1, length len2-1, so in the end
the check was off by one. But let's require the terminating nul too, even if
we don't access it.
CID #1383035.
This is external data, even if trusted. We should not assert on it, but verify
and return proper error instead, which assert_return does. In particular,
write(2) says that a partial write could occur when interupted by a signal.
When compiled with asserts disabled, we could access memory outside of the
allocated buffer.
CID #1237671.
Follow-up for 1a96c8e1cc.
We were inconsitently using them in some cases, but in majority not.
Using assignment in assert_se is very common, not an exception like in
'if', so let's drop the extra parens everywhere.
The leak can be reproduced by running systemd-path --suffix .tmp under valgrind or asan:
$ ./build/systemd-path --suffix .tmp search-binaries
/usr/local/bin/.tmp:/usr/bin/.tmp:/usr/local/sbin/.tmp:/usr/sbin/.tmp:/home/vagrant/.local/bin/.tmp:/home/vagrant/bin/.tmp
=================================================================
==19177==ERROR: LeakSanitizer: detected memory leaks
Direct leak of 56 byte(s) in 1 object(s) allocated from:
*0 0x7fd6adf72850 in malloc (/lib64/libasan.so.4+0xde850)
*1 0x7fd6ad2b93d2 in malloc_multiply ../src/basic/alloc-util.h:69
*2 0x7fd6ad2bafd2 in strv_split ../src/basic/strv.c:269
*3 0x7fd6ad42ba67 in search_from_environment ../src/libsystemd/sd-path/sd-path.c:409
*4 0x7fd6ad42bffe in get_search ../src/libsystemd/sd-path/sd-path.c:482
*5 0x7fd6ad42c55b in sd_path_search ../src/libsystemd/sd-path/sd-path.c:607
*6 0x7fd6ad42b3a2 in sd_path_home ../src/libsystemd/sd-path/sd-path.c:348
*7 0x55f59c65ebea in print_home ../src/path/path.c:97
*8 0x55f59c65f157 in main ../src/path/path.c:177
*9 0x7fd6abaea009 in __libc_start_main (/lib64/libc.so.6+0x21009)
Indirect leak of 68 byte(s) in 5 object(s) allocated from:
*0 0x7fd6adf72850 in malloc (/lib64/libasan.so.4+0xde850)
*1 0x7fd6abb5f689 in strndup (/lib64/libc.so.6+0x96689)
Indirect leak of 25 byte(s) in 1 object(s) allocated from:
*0 0x7fd6adf72850 in malloc (/lib64/libasan.so.4+0xde850)
*1 0x7fd6abb5f689 in strndup (/lib64/libc.so.6+0x96689)
*2 0x6c2e2f746e617266 (<unknown module>)
SUMMARY: AddressSanitizer: 149 byte(s) leaked in 7 allocation(s).
Previously we were a bit sloppy with the index and size types of arrays,
we'd regularly use unsigned. While I don't think this ever resulted in
real issues I think we should be more careful there and follow a
stricter regime: unless there's a strong reason not to use size_t for
array sizes and indexes, size_t it should be. Any allocations we do
ultimately will use size_t anyway, and converting forth and back between
unsigned and size_t will always be a source of problems.
Note that on 32bit machines "unsigned" and "size_t" are equivalent, and
on 64bit machines our arrays shouldn't grow that large anyway, and if
they do we have a problem, however that kind of overly large allocation
we have protections for usually, but for overflows we do not have that
so much, hence let's add it.
So yeah, it's a story of the current code being already "good enough",
but I think some extra type hygiene is better.
This patch tries to be comprehensive, but it probably isn't and I missed
a few cases. But I guess we can cover that later as we notice it. Among
smaller fixes, this changes:
1. strv_length()' return type becomes size_t
2. the unit file changes array size becomes size_t
3. DNS answer and query array sizes become size_t
Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=76745
This cleans up handling of MTU values across the codebase. Previously
MTU values where stored sometimes in uint32_t, sometimes in uint16_t,
sometimes unsigned and sometimes in size_t. This now unifies this to
uint32_t across the codebase, as that's what netlink spits out, and what
the majority was already using.
Also, all MTU parameters are now parsed with config_parse_mtu() and
config_parse_ipv6_mtu() is dropped as it is now unneeded.
(Note there is one exception for the MTU typing: in the DCHPv4 code we
continue to process the MTU as uint16_t value, as it is encoded like
that in the protocol, and it's probably better stay close to the
protocol there.)
Double newlines (i.e. one empty lines) are great to structure code. But
let's avoid triple newlines (i.e. two empty lines), quadruple newlines,
quintuple newlines, …, that's just spurious whitespace.
It's an easy way to drop 121 lines of code, and keeps the coding style
of our sources a bit tigther.
Files which are installed as-is (any .service and other unit files, .conf
files, .policy files, etc), are left as is. My assumption is that SPDX
identifiers are not yet that well known, so it's better to retain the
extended header to avoid any doubt.
I also kept any copyright lines. We can probably remove them, but it'd nice to
obtain explicit acks from all involved authors before doing that.
sd_bus_open/sd_bus_open_system/sd_bus_open_user are convenient, but
don't allow the description to be set. After they return, the bus is
is already started, and sd_bus_set_description() fails with -EBUSY.
It would be possible to allow sd_bus_set_description() to update the
description "live", but messages are already emitted from sd_bus_open
functions, so it's better to allow the description to be set in
sd_bus_open/sd_bus_open_system/sd_bus_open_user.
Fixes message like:
Bus n/a: changing state UNSET → OPENING
This macro will read a pointer of any type, return it, and set the
pointer to NULL. This is useful as an explicit concept of passing
ownership of a memory area between pointers.
This takes inspiration from Rust:
https://doc.rust-lang.org/std/option/enum.Option.html#method.take
and was suggested by Alan Jenkins (@sourcejedi).
It drops ~160 lines of code from our codebase, which makes me like it.
Also, I think it clarifies passing of ownership, and thus helps
readability a bit (at least for the initiated who know the new macro)
Some versions of asan report the following false positive
when strict_string_checks=1 is passed:
=================================================================
==3297==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x7f64e4090286 bp 0x7ffe46acd9a0 sp 0x7ffe46acd118 T0)
==3297==The signal is caused by a READ memory access.
==3297==Hint: address points to the zero page.
#0 0x7f64e4090285 in __strlen_sse2 (/lib64/libc.so.6+0xaa285)
#1 0x7f64e5a51e46 (/lib64/libasan.so.4+0x41e46)
#2 0x7f64e4e5e3a0 (/lib64/libglib-2.0.so.0+0x383a0)
#3 0x7f64e4e5e536 in g_dgettext (/lib64/libglib-2.0.so.0+0x38536)
#4 0x7f64e48fac5f (/lib64/libgio-2.0.so.0+0xc1c5f)
#5 0x7f64e4c03978 in g_type_class_ref (/lib64/libgobject-2.0.so.0+0x30978)
#6 0x7f64e4be9567 in g_object_new_with_properties (/lib64/libgobject-2.0.so.0+0x16567)
#7 0x7f64e4be9fd0 in g_object_new (/lib64/libgobject-2.0.so.0+0x16fd0)
#8 0x7f64e48fd43e in g_dbus_message_new_from_blob (/lib64/libgio-2.0.so.0+0xc443e)
#9 0x564a6aa0de52 in main ../src/libsystemd/sd-bus/test-bus-marshal.c:228
#10 0x7f64e4007009 in __libc_start_main (/lib64/libc.so.6+0x21009)
#11 0x564a6aa0a569 in _start (/home/vagrant/systemd/build/test-bus-marshal+0x5569)
AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV (/lib64/libc.so.6+0xaa285) in __strlen_sse2
==3297==ABORTING
It's an external library and errors in external libraries are generally not very
useful for looking for internal bugs.
It would be better not to change the code and use standard suppression
techinques decribed at
https://clang.llvm.org/docs/AddressSanitizer.html#suppressing-reports-in-external-libraries,
but, unfortunaley, none of them seems to be able to suppress fatal errors in asan intself.
