We had atomic counters, but all other operations were non-serialized. This
means that concurrent access to the bus object was only safe if _all_ threads
were doing read-only access. Even sending of messages from threads would not be
possible, because after sending of the message we usually want to remove it
from the send queue in the bus object, which would race. Let's just kill this.
Before this commit bus messages had a single reference count: when it
reached zero the message would be freed. This simple approach meant a
cyclic dependency was typically seen: a message that was enqueued in a
bus connection object would reference the bus connection object but also
itself be referenced by the bus connection object. So far out strategy
to avoid cases like this was: make sure to process the bus connection
regularly so that messages don#t stay queued, and at exit flush/close
the connection so that the message queued would be emptied, and thus the
cyclic dependencies resolved. Im many cases this isn't done properly
however.
With this change, let's address the issue more systematically: let's
break the reference cycle. Specifically, there are now two types of
references to a bus message:
1. A regular one, which keeps both the message and the bus object it is
associated with pinned.
2. A "queue" reference, which is weaker: it pins the message, but not
the bus object it is associated with.
The idea is then that regular user handling uses regular references, but
when a message is enqueued on its connection, then this takes a "queue"
reference instead. This then means that a queued message doesn't imply
the connection itself remains pinned, only regular references to the
connection or a message associated with it do. Thus, if we end up in the
situation where a user allocates a bus and a message and enqueues the
latter in the former and drops all refs to both, then this will detect
this case and free both.
Note that this scheme isn't perfect, it only covers references between
messages and the busses they are associated with. If OTOH a bus message
is enqueued on a different bus than it is associated with cyclic deps
cannot be recognized with this simple algorithm, and thus if you enqueue
a message associated with a bus A on a bus B, and another message
associated with bus B on a bus A, a cyclic ref will be in effect and not
be discovered. However, given that this is an exotic case (though one
that happens, consider systemd-bus-stdio-bridge), it should be OK not to
cover with this, and people have to explicit flush all queues on exit in
that case.
Note that this commit only establishes the separate reference counters
per message. A follow-up commit will start making use of this from the
bus connection object.
Don't try to be smart, don't bypass the ref counting logic if there's no
real reason to.
This matters if we want to tweak the ref counting logic later.
Let's make sure our own code follows coding style and initializes all
return values on all types of success (and leaves it uninitialized in
all types of failure).
Let's do this like we usually do and size arrays with size_t.
We already do this for the "allocated" counter correctly, and externally
we expose the queue sizes as uint64_t anyway, hence there's really no
point in usigned "unsigned" internally.
Apparently this happens IRL. Let's carefully deal with issues like this:
when we overrun, let's not go back to zero but instead leave the highest
cookie bit set. We use that as indication that we are in "overrun
territory", and then are particularly careful with checking cookies,
i.e. that they haven't been used for still outstanding replies yet. This
should retain the quick cookie generation behaviour we used to have, but
permits dealing with overruns.
Replaces: #11804Fixes: #11809
Paths are limited to BUS_PATH_SIZE_MAX but the maximum size is anyway too big
to be allocated on the stack, so let's switch to the heap where there is a
clear way to understand if the allocation fails.
Even though the dbus specification does not enforce any length limit on the
path of a dbus message, having to analyze too long strings in PID1 may be
time-consuming and it may have security impacts.
In any case, the limit is set so high that real-life applications should not
have a problem with it.
dbus-daemon might have a slightly different idea of what a valid msg is
than us (for example regarding valid msg and field sizes). Let's hence
try to proceed if we can and thus drop messages rather than fail the
connection if we fail to validate a message.
Hopefully the differences in what is considered valid are not visible
for real-life usecases, but are specific to exploit attempts only.
It's similar to sd_bus_flush_close_unref() but doesn't do the flushing.
This is useful since this will still discnnect the connection properly
but not synchronously wait for the peer to take our messages.
Primary usecase is within _cleanup_() expressions where synchronously
waiting on the peer is not OK.
From WordNet (r) 3.0 (2006) [wn]:
time-out
n 1: a brief suspension of play; "each team has two time-outs left"
From The Free On-line Dictionary of Computing (18 March 2015) [foldoc]:
timeout
A period of time after which an error condition is raised if
some event has not occured. A common example is sending a
message. If the receiver does not acknowledge the message
within some preset timeout period, a transmission error is
assumed to have occured.
This has been irritating me for quite a while: let's prefix these enum
values with a common prefix, like we do for almost all other enums.
No change in behaviour, just some renaming.
This cleans up a bit how we set up things for the ELF section magic:
1. Let's always use our gcc macros, instead of __attribute__ directly
2. Align our structures to sizeof(void*), i.e. the pointer size, rather
than a fixed 8 or __BIGGEST_ALIGNMENT__. The former is unnecessarily
high for 32bit systems, the latter too high for 64bit systems. gcc
seems to use ptr alignment for static variables itself, hence this
should be good enough for us too. Moreover, the Linux kernel also
uses pointer alginment for all its ELF section registration magic,
hence this should be good enough for us too.
3. Let's always prefix the sections we create ourself with SYSTEMD_,
just to make clear where they come from.
4. Always align the pointer we start from when iterating through these
lists. This should be unnecessary, but makes things nicely
systematic, as we'll align all pointers we use to access these
sections properly.
This splits out a bunch of functions from fileio.c that have to do with
temporary files. Simply to make the header files a bit shorter, and to
group things more nicely.
No code changes, just some rearranging of source files.
Whenever we invoke external, foreign code from code that has
RLIMIT_NOFILE's soft limit bumped to high values, revert it to 1024
first. This is a safety precaution for compatibility with programs using
select() which cannot operate with fds > 1024.
This commit adds the call to rlimit_nofile_safe() to all invocations of
exec{v,ve,l}() and friends that either are in code that we know runs
with RLIMIT_NOFILE bumped up (which is PID 1 and all journal code for
starters) or that is part of shared code that might end up there.
The calls are placed as early as we can in processes invoking a flavour
of execve(), but after the last time we do fd manipulations, so that we
can still take benefit of the high fd limits for that.
If the caller asks for a unique name, but the input name is already
unique we can shortcut things and just use that, as long as we make at
least one bus call to validate it.
Ideally, coccinelle would strip unnecessary braces too. But I do not see any
option in coccinelle for this, so instead, I edited the patch text using
search&replace to remove the braces. Unfortunately this is not fully automatic,
in particular it didn't deal well with if-else-if-else blocks and ifdefs, so
there is an increased likelikehood be some bugs in such spots.
I also removed part of the patch that coccinelle generated for udev, where we
returns -1 for failure. This should be fixed independently.
As suggest here:
https://gcc.gnu.org/onlinedocs/gcc/Attribute-Syntax.html#Attribute-Syntax
"You may optionally specify attribute names with ‘__’ preceding and
following the name. This allows you to use them in header files without
being concerned about a possible macro of the same name. For example,
you may use the attribute name __noreturn__ instead of noreturn. "
asan doesn't like it if we use strndup() (i.e. a string function) on a
non-NULL terminated buffer (i.e. something that isn't really a string).
Let's hence use memdup_suffix0() instead of strndup(), which is more
appropriate for binary data that is to become a string.
Fixes: #10385
