Michal Sekletar [Tue, 14 Nov 2017 18:29:37 +0000 (19:29 +0100)]
udev: net_setup_link: don't error out when we couldn't apply link config (#7328)
It is possible that kernel will reject our netlink message that
configures the link. However, we should always make sure that interface
will be named properly otherwise we can leave interfaces having
unpredictable kernel names. Thus we don't return early and continue to
export name and link file properties.
Alan Jenkins [Fri, 15 Sep 2017 16:35:02 +0000 (17:35 +0100)]
logind: fix SetLinger to authorize by client's effective User ID
SetLinger is authorized by the PolicyKit action "set-self-linger", if it is
not passed an explicit UID.
According to comments we were determining the default UID from the client's
session. However, user processes e.g. which are run from a terminal
emulator do not necessarily belong to a session scope unit. They may
equally be started from the systemd user manager [1][2]. Actually the
comment was wrong, and it would also have worked for processes
started from the systemd user manager.
Nevertheless it seems to involve fetching "augmented credentials" i.e.
it's using a racy method, so we shouldn't have been authenticating based
on it.
We could change the default UID, but that raises issues especially for
consistency between the methods. Instead we can just use the clients
effective UID for authorization.
This commit also fixes `loginctl enable-linger $USER` to match the docs
that say it was equivalent to `loginctl enable-linger` (given that $USER
matches the callers user and owner_uid). Previously, the former would not
have suceeded for unpriviliged users in the default configuration.
[1] It seems the main meaning of per-session scopes is tracking the PAM
login process. Killing that provokes logind to revoke device access. Less
circularly, killing it provokes getty to hangup the TTY.
[2] User units may be started with an environment which includes
XDG_SESSION_ID (presuambly GNOME does this?). Or not.
Alan Jenkins [Mon, 18 Sep 2017 17:04:59 +0000 (18:04 +0100)]
loginctl: enable-linger does not need fallback to XDG_SESSION_ID
To maintain consistency with `loginctl user-status`, drop the fallback to
XDG_SESSION_ID for `loginctl enable-linger`. The fallback was unnecessary
and also incorrect: it passed the numeric value of the session identifier
as a UID value.
Alan Jenkins [Sun, 17 Sep 2017 14:49:12 +0000 (15:49 +0100)]
logind: comment use of *_get_session()
The manpages tell that such calls have quite limited meaning. logind has
a few in the implementation of what remains of the session concept.
At the same time, logind basically exposes sd_pid_get_session() as public
API. This is absolutely required, to retain compatability e.g. with Xorg.
But client code will work in more situations if it avoids assuming that it
runs in a session itself.
Its use inside the login session could be replaced with $XDG_SESSION_ID
(which pam_systemd sets). I don't know whether it would be useful to
change Xorg at this point or not. But if you were building something new,
you would think about whether you want to support running it in a systemd
service.
Comment these logind API features, acknowledging the reason they exist is
based in history. I.e. help readers avoid drawing implications from their
existence which apply to history, but not the current general case.
Finally, searching these revealed a call to sd_pid_get_session() in
implementing some types of logind inhibitors. So these inhibitors don't
work as intended when taken from inside a systemd user service :(. Comment
this as well, deferring it as ticket #6852.
Alan Jenkins [Tue, 17 Oct 2017 15:03:58 +0000 (16:03 +0100)]
logind: more specific error message for unknown users
If you try to run `loginctl user-status` on a non-logged in user to see
whether "Linger" is enabled, it doesn't work.
If you're already an expert in logind, the fact that the user is considered
unknown actually tells you the user is not lingering. So, probably they
they do not have lingering enabled. I think we can point towards this
without being misleading.
I also reword it because I thought it was slightly confusing to run
`loginctl user-status root` and get an error back about "User 0". Try to
be more specific, that it is "User ID 0".
Alan Jenkins [Sat, 14 Oct 2017 08:25:56 +0000 (09:25 +0100)]
logind: "self" objects which do not apply - return specific error messages
It's confusing that the bus API has aliases like "session/self" that return
an error based on ENXIO, when it also has methods that return e.g.
NO_SESSION_FOR_PID for the same problem. The latter kind of error includes
more specifically helpful messages.
"user/self" is the odd one out; it returns a generic UnknownObject error
when it is not applicable to the caller. It's not clear whether this was
intentional, but at first I thought it was more correct. More
specifically, user_object_find() was returning 0 for "user/self", in the
same situations (more or less) where user_node_enumerator() was omitting
"user/self". I thought that was a good idea, because returning e.g. -ENXIO instead
suggested that there _is_ something specific on that path. And it could be
confused with errors of the method being called.
Therefore I suggested changing the enumerator, always admitting that there
is a handler for the path "foo/self", but returning a specific error when
queried. However this interacts poorly with tools like D-Feet or `busctl`.
In either tool, looking at logind would show an error message, and then go
on to omit "user/self" in the normal listing. These tools are very useful,
so we don't want to interfere with them.
I think we can change the error codes without causing problems. The self
objects were not listed in the documentation. They have been suggested to
other projects - but without reference to error reporting. "seat/self" is
used by various Wayland compositors for VT switching, but they don't appear
to reference specific errors.
We _could_ insist on the link between enumeration and UnknownObject, and
standardize on that as the error for the aliases. But I'm not aware of any
practical complaints, that we returned an error from an object that didn't
exist.
Instead, let's unify the codepaths for "user/self" vs GetUserByPid(0) etc.
We will return the most helpful error message we can think of, if the
object does not exist. E.g. for "session/self", we might return an error
that the caller does not belong to a session. If one of the compositors is
ever simplified to use "session/self" in initialization, users would be
able to trigger such errors (e.g. run `gnome-shell` inside gnome-terminal).
The message text will most likely be logged. The user might not know what
the "session" is, but at least we'll be pointing towards the right
questions. I think it should also be clearer for development / debugging.
Unifying the code paths is also slightly helpful for auditing / marking
calls to sd_bus_creds_get_session() in subsequent commits.
Alan Jenkins [Sat, 7 Oct 2017 11:24:02 +0000 (12:24 +0100)]
logind: remove an obscure dbus error from GetSessionByPID(0) and friends
GetSessionByPID(0) can fail with NO_SESSION_FOR_PID. More obscurely, if
the session is abandoned, it can return NO_SUCH_SESSION. It is not clear
that the latter was intended. The message associated with the former,
hints that this was overlooked.
We don't have a document enumerating the errors. Any specific
error-handling in client code, e.g. translated messages, would also be
liable to overlook the more obscure error code.
I can't see any equivalent condition for GetUserByPID(0). On the other
hand, the code did not return NO_USER_FOR_PID where it probably should.
The relevant code is right next to that for GetSessionByPID(0), so it will
be simpler to understand if both follow the same pattern.
Yu Watanabe [Tue, 14 Nov 2017 08:01:21 +0000 (17:01 +0900)]
core/dbus-unit: add property_get_requires_mounts_for() to send correct message (#7322)
PR #7186 changes requires_mounts_for from strv to Hashmap.
So, it is necessary to implement a function for getting the property RequiresMountsFor=.
This introduces property_get_requires_mounts_for() which reads the Hashmap
and sends messages to bus.
Zeal Jagannatha [Tue, 14 Nov 2017 07:11:41 +0000 (23:11 -0800)]
Fix error message when binding files (#7196)
If you use machinectl to bind a file into a container, it responds with a confusing error message about a temporary directory not being a directory.
I just swapped it to error with the source that was passed, rather than the tmpdir.
It would also be nice to be able to bind files, but that's a separate issue (#7195).
Before the change:
root@epona /var/lib/sandbox $ cat bar/foo
Hello world!
root@epona /var/lib/sandbox $ machinectl bind testing /var/lib/sandbox/bar/foo /foo
Failed to bind mount: Failed to overmount /tmp/propagate.W5TNsj/mount: Not a directory
After the change:
root@epona /var/lib/sandbox $ machinectl bind testing /var/lib/sandbox/bar/foo /foo
Failed to bind mount: Failed to overmount /var/lib/sandbox/bar/foo: Not a directory
core: only warn about BPF/cgroup missing once per runtime (#7319)
Let's reduce the amount of noise a bit, there's little point in
complaining loudly about every single unit like this, let's complain
only about the first one, and then downgrade the log level to LOG_DEBUG
for the other cases.
i18n: drop intltool use, use meson's merge_file directly
This didn't work during the initial conversion to meson, but should now.
A sufficiently new polkit is also required, for the .its rules files.
Note that https://github.com/mesonbuild/meson/blob/master/docs/markdown/i18n-module.md
says that 'install' argument was added in meson 0.43.0. If this is accurate,
warnigs might be generated with older mesons. Fedora has 0.43.0 across the
board, but other distros probably don't, but I guess that a warning is
prefereable to having to update do latest meson.
The advantages are:
- one less dependency (intltool)
- using the generic implementation instead of our open-coded calls
- we don't need to use the fake "_" prefixes in XML
man: rework systemd-vconsole-setup man page a bit (#7312)
It's not systemd that invokes the service internally as needed, it's
systemd-localed. Correct that.
Also, stop using the word "helper". To me a "helper" constitutes
something internal, not official API. I doubt systemd-vconsole-setup
really matches that description though, hence let's better avoid the
term.
Also, clean up some other wording, and be less imbiguous, by suggesting
a single command to apply vconsole.conf instead of two.
install: drop redundant printing of unit name (#7296)
We already print it as part of log_syntax() internal logic, don't print
it again, and in particular, don't print it at the end of log line, such
a strange place.
bpf-firewall: properly handle kernels where BPF cgroup is disabled but TRIE maps are enabled (#7298)
So far, we assumed that kernels where TRIE was on also supported
BPF/cgroup stuff. That's not a correct assumption to make, hence check
for both features separately.
core: rework the Delegate= unit file setting to take a list of controller names
Previously it was not possible to select which controllers to enable for
a unit where Delegate=yes was set, as all controllers were enabled. With
this change, this is made configurable, and thus delegation units can
pick specifically what they want to manage themselves, and what they
don't care about.
namespace: set up OS hierarchy only after mounting the new root, not before
Otherwise it's a pointless excercise, as we'll set up an empty directory
tree that's never going to be used.
Hence, let's move this around a bit, so that we do the basesystem
initialization exactly when RootImage= or RootDirectory= are used, but
not otherwise.
shared/seccomp: skip pkey_mprotect protections if the syscall is unknown
When compiling with an old kernel on architectures for which the
number is not defined in missing.h, a warning is generated in missing.h.
Let's just skip the protection in this case, to allow build to proceed.
shared/seccomp: disallow pkey_mprotect the same as mprotect for W^X mappings (#7295)
MemoryDenyWriteExecution policy could be be bypassed by using pkey_mprotect
instead of mprotect to create an executable writable mapping.
The impact is mitigated by the fact that the man page says "Note that this
feature is fully available on x86-64, and partially on x86", so hopefully
people do not rely on it as a sole security measure.
Found by Karin Hossen and Thomas Imbert from Sogeti ESEC R&D.
networkd: improve interface rename log message a bit (#7299)
Let's clarify that it's not networkd that renames interfaces, but
something else (for example, udev's link builtin based on .link files)
This doesn't change any logic, it just rewords the message a bit, to
clarify that we only log this for informational purposes, not because we
execute the rename operation ourselves.
Merge pull request #7301 from poettering/loginctl-ellipsize
Fix loginctl seat sysfs tree ellipsation logic.
Simple reproducer:
loginctl --full seat-status seat0|cat
→ after this PR, all lines are shown in full. Before, lines were ellipsized to terminal width.
Yu Watanabe [Sat, 11 Nov 2017 12:35:49 +0000 (21:35 +0900)]
core: add support to specify errno in SystemCallFilter=
This makes each system call in SystemCallFilter= blacklist optionally
takes errno name or number after a colon. The errno takes precedence
over the one given by SystemCallErrorNumber=.
loginctl: rework sysfs tree dump, to honour --full and friends
Let's hook up the sysfs tree output with the output flags logic,
already used when dumping log lines or process trees. This way we get
very similar output handling for line breaking/ellipsation in all three
outputs of structured data.
string-util: when ellipsizing to a length if (size_t) -1, become a NOP
Let's say that (size_t) -1 (i.e. SIZE_T_MAX) is equivalent to
"unbounded" ellipsation, i.e. ellipsation as NOP. In which case the
relevant functions become little more than strdup()/strndup().
This is useful to simplify caller code in case we want to turn off
ellipsation in certain code paths with minimal caller-side handling for
this.
merge two lines in our get_output_flags() functions
loginctl, machinectl, systemctl all have very similar implementations of
a get_output_flags() functions. Simplify it by merging two lines that
set the same flag.
core: make "tmpfs" dependencies on swapfs a "default" dep, not an "implicit"
There should be a way to turn this logic of, and DefaultDependencies=
appears to be the right option for that, hence let's downgrade this
dependency type from "implicit" to "default, and thus honour
DefaultDependencies=.
This also drops mount_get_fstype() as we only have a single user needing
this now.
core: remove SYSTEMD_WANTS udev property configured dependencies at the right moment
Previously dependencies configured with SYSTEMD_WANTS would be collected
on a device unit as long as it was loaded. let's fix that, and remove
dependencies again when SYTEMD_WANTS changes.
This replaces the dependencies Set* objects by Hashmap* objects, where
the key is the depending Unit, and the value is a bitmask encoding why
the specific dependency was created.
The bitmask contains a number of different, defined bits, that indicate
why dependencies exist, for example whether they are created due to
explicitly configured deps in files, by udev rules or implicitly.
Note that memory usage is not increased by this change, even though we
store more information, as we manage to encode the bit mask inside the
value pointer each Hashmap entry contains.
Why this all? When we know how a dependency came to be, we can update
dependencies correctly when a configuration source changes but others
are left unaltered. Specifically:
1. We can fix UDEV_WANTS dependency generation: so far we kept adding
dependencies configured that way, but if a device lost such a
dependency we couldn't them again as there was no scheme for removing
of dependencies in place.
2. We can implement "pin-pointed" reload of unit files. If we know what
dependencies were created as result of configuration in a unit file,
then we know what to flush out when we want to reload it.
3. It's useful for debugging: "systemd-analyze dump" now shows
this information, helping substantially with understanding how
systemd's dependency tree came to be the way it came to be.
mkosi: fix build script to use right sysvinit path
On Fedora /etc/init.d is a symlink to /etc/rc.d/init.d. Our build
scripts default to /etc/init.d since that is the LSB default. Let's make
sure the build script thus follows the symlink correctly and configures
to path explicitly, since otherwise our build artifacts in $DESTDIR are
incompatible with the setup we actually need for Fedora.
Alan Jenkins [Fri, 10 Nov 2017 14:57:52 +0000 (14:57 +0000)]
core: add missing error_message cases (#6911)
We neglected to set error_message for errors which occur _after_ the
`finish` label. These fatal errors only happen in paths where `finish`
was reached successfully, i.e. error_message has not already been set
(and this analysis is simple enough that this need not cause too much
headaches. Also our new assignments to error_message come immediately
after execve() calls, which would have lost the error_message if it had
been set).
Also print a status message when we fail to exec init, otherwise the only
sign the user will see is `# ` :).
This addresses the lack of error messages pointed out in issue #6827.
basic/hashmap: add cleanup of memory pools (#7164)
It was dropped in 89439d4fc0d29f04ac68432fd06ab84bc4e36e20. As a result, every
process that uses a hashmap allocates and then leaks the hashmap mempools.
The mempools are only allocated in the main thread, but we don't know where
the memory is used.
So let's check if we are the last thread and free the mempools then. This is
fairly heavy, because /proc/self/status has to be opened and parsed, but we do
it only when compiled for valgrind, i.e. not by default, and compared to running
under valgrind or asan, the extra cost is acceptable. The big advantage is that
we don't have to think or filter out this false positive.
As a micro-opt, cleanup is attempted only in the main thread. We could allow
any thread to check if it is the last one and perform cleanup, but that'd mean
that we'd have to _do_ the check in every thread. We don't use threads like
that, our non-main threads are always short-lived, so let's just accept the
possibility that we'll leak memory if a thread survives. The check is also
non-atomic, but it's called in a destructor of the main thread _and_ we do
cleanup only when there are no other threads, so the risk of some library
suddenly spawning another thread is very low. All in all, this is not perfect,
but should work in 999‰ of cases.
Fixes the following valgrind warning:
==22564== HEAP SUMMARY:
==22564== in use at exit: 8,192 bytes in 2 blocks
==22564== total heap usage: 243 allocs, 241 frees, 151,905 bytes allocated
==22564==
==22564== 4,096 bytes in 1 blocks are still reachable in loss record 1 of 2
==22564== at 0x4C2FB6B: malloc (vg_replace_malloc.c:299)
==22564== by 0x4F08A8C: mempool_alloc_tile (mempool.c:62)
==22564== by 0x4F08B16: mempool_alloc0_tile (mempool.c:81)
==22564== by 0x4EF8DE0: hashmap_base_new (hashmap.c:748)
==22564== by 0x4EF8ED9: internal_hashmap_new (hashmap.c:782)
==22564== by 0x11045D: test_hashmap_copy (test-hashmap-plain.c:87)
==22564== by 0x115722: test_hashmap_funcs (test-hashmap-plain.c:914)
==22564== by 0x10FC9D: main (test-hashmap.c:60)
==22564==
==22564== 4,096 bytes in 1 blocks are still reachable in loss record 2 of 2
==22564== at 0x4C2FB6B: malloc (vg_replace_malloc.c:299)
==22564== by 0x4F08A8C: mempool_alloc_tile (mempool.c:62)
==22564== by 0x4F08B16: mempool_alloc0_tile (mempool.c:81)
==22564== by 0x4EF8DE0: hashmap_base_new (hashmap.c:748)
==22564== by 0x4EF8EF8: internal_ordered_hashmap_new (hashmap.c:786)
==22564== by 0x10A2A0: test_ordered_hashmap_copy (test-hashmap-ordered.c:89)
==22564== by 0x10F70F: test_ordered_hashmap_funcs (test-hashmap-ordered.c:916)
==22564== by 0x10FCA2: main (test-hashmap.c:61)
==22564==
==22564== LEAK SUMMARY:
==22564== definitely lost: 0 bytes in 0 blocks
==22564== indirectly lost: 0 bytes in 0 blocks
==22564== possibly lost: 0 bytes in 0 blocks
==22564== still reachable: 8,192 bytes in 2 blocks
==22564== suppressed: 0 bytes in 0 blocks
Yu Watanabe [Fri, 10 Nov 2017 09:15:55 +0000 (18:15 +0900)]
core/load-fragment: add RemoveIPC= (#7288)
PR #3865 introduced RemoveIPC= but the option is not listed in
load-fragment-gperf.gperf. So, the option could be used only via d-bus.
This adds RemoveIPC= in load-fragment-gperf.gperf. Then, now we can
set the option in unit files.