This also makes sure the control buffer is properly aligned. This
matters, as otherwise the control buffer might not be aligned and the
cmsg buffer counting might be off. The incorrect alignment is becoming
visible by using recvmsg_safe() as we suddenly notice the MSG_CTRUNC bit
set because of this.
That said, apparently this isn't enough to make this work on all
kernels. Since I couldn't figure this out, we now add 1K to the buffer
to be sure. We do this once already, also for a pktinfo structure
(though an IPv4/IPv6) one. I am puzzled by this, but this shouldn't
matter much. it works locally just fine, except for those ubuntu CI
kernels...
While we are at it, make some other changes too, to simplify and
modernize the function.
Joel Shapiro [Sat, 9 May 2020 05:38:41 +0000 (00:38 -0500)]
Fix misuse of PAM_PROMPT_ECHO_OFF in systemd-homed
Previously pam_systemd_home.so was relying on `PAM_PROMPT_ECHO_OFF` to
display error messages to the user and also display the next prompt.
`PAM_PROMPT_ECHO_OFF` was never meant as a way to convey information to
the user, and following the example set in pam_unix.so you can see that
it's meant to _only_ display the prompt. Details about why the
authentication failed should be done in a `PAM_ERROR_MSG` before
displaying a short prompt as per usual using `PAM_PROMPT_ECHO_OFF`.
shared/ethtool-util: hush gcc warnings about array bounds
[127/1355] Compiling C object 'src/shared/5afaae1@@systemd-shared-245@sta/ethtool-util.c.o'
../src/shared/ethtool-util.c: In function ‘ethtool_get_permanent_macaddr’:
../src/shared/ethtool-util.c:260:60: warning: array subscript 5 is outside the bounds of an interior zero-length array ‘__u8[0]’ {aka ‘unsigned char[]’} [-Wzero-length-bounds]
260 | ret->ether_addr_octet[i] = epaddr.addr.data[i];
| ~~~~~~~~~~~~~~~~^~~
In file included from ../src/shared/ethtool-util.c:5:
../src/shared/linux/ethtool.h:704:7: note: while referencing ‘data’
704 | __u8 data[0];
| ^~~~
../src/shared/ethtool-util.c: In function ‘ethtool_set_features’:
../src/shared/ethtool-util.c:488:31: warning: array subscript 0 is outside the bounds of an interior zero-length array ‘__u32[0]’ {aka ‘unsigned int[]’} [-Wzero-length-bounds]
488 | len = buffer.info.data[0];
| ~~~~~~~~~~~~~~~~^~~
In file included from ../src/shared/ethtool-util.c:5:
../src/shared/linux/ethtool.h:631:8: note: while referencing ‘data’
631 | __u32 data[0];
| ^~~~
The kernel should not define the length of the array, but it does. We can't fix
that, so let's use a cast to avoid the warning.
For https://github.com/systemd/systemd/issues/6119#issuecomment-626073743.
v2:
- use #pragma instead of a cast. It seems the cast only works in some cases, and
gcc is "smart" enough to see beyond the cast. Unfortunately clang does not support
this warning, so we need to do a config check whether to try to suppress.
man: clarify that exit status name mappings are unaffected by SuccessExitStatus=
Fixes: #15757
(Note there's quite some confusion regarding "exit status" vs. "exit
code" in the docs here. We should clean this up fully one day. This
change tries to fix some occasions of the wrong use, but not all.)
man: mention the exclamation mark and minus sign literally, to make things searchable
I was looking for the explanation for the exclamation mark in the text,
and couldn't find it, searching for "!". Let's make this easier, and
indicate the character meant.
random-util: make use of GRND_INSECURE when it is defined
kernel 5.6 added support for a new flag for getrandom(): GRND_INSECURE.
If we set it we can get some random data out of the kernel random pool,
even if it is not yet initializated. This is great for us to initialize
hash table seeds and such, where it is OK if they are crap initially. We
used RDRAND for these cases so far, but RDRAND is only available on
newer CPUs and some archs. Let's now use GRND_INSECURE for these cases
as well, which means we won't needlessly delay boot anymore even on
archs/CPUs that do not have RDRAND.
Of course we never set this flag when generating crypto keys or uuids.
Which makes it different from RDRAND for us (and is the reason I think
we should keep explicit RDRAND support in): RDRAND we don't trust enough
for crypto keys. But we do trust it enough for UUIDs.
наб [Thu, 7 May 2020 19:34:39 +0000 (21:34 +0200)]
networkctl: use uint64_t for link speed throughout
format-table used size_t/uint64_t interchangeably for TABLE_BPS,
and ethtool-util used SIZE_MAX to indicate SPEED_UNKNOWN,
which worked only on ABIs with 64-bit pointers.
For example, the tg3 driver returns SPEED_UNKNOWN with no link (cf.
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/net/ethernet/broadcom/tg3.c?id=3eb2efbea193789397c36f52b17d8692ac79bf68#n12190)
which on x32 (and other 32-bit ABIs, presumably) caused
"networkctl status" to mark it with "Speed: 4Gbps":
Apart from running CIFuzz for each relevant PR, let's run it
unconditionally for each push to master to detect possible issues
(caused by ignored PRs, etc.).
Hans de Goede [Thu, 7 May 2020 21:16:21 +0000 (23:16 +0200)]
hwdb: Fix accel orientation quirk for Medion Akoya E2215T MD60198
Commit 3d864658ea01 ("hwdb: assume all Medion Akoya E-models have the
same matrix"), unified the entries for most Medion Akoya E* modes,
including the entry for the Medion Akoya E2215T MD60198.
But the accelerometer on the E2215T has an ACPI-id of KIOX000A, rather
then KIOX010A as the other models have, so that commit broke the
ACCEL_MOUNT_MATRIX for the E2215T.
Add a separate match for the E2215T so that it gets the correct
ACCEL_MOUNT_MATRIX again.
homed: tweak logic for picking a default storage and file system
Let's make the logic a bit smarter: if we detect that /home is
encrypted, let's avoid double encryption and prefer plain
directory/subvolumes instead of our regular luks images.
Also, allow configuration go storage/file system via an env var passed
to homework. In a later commit, let's then change homed to initialize
that env var from a config file setting, when invoking homework.
Michal Sekletar [Thu, 7 May 2020 15:23:30 +0000 (17:23 +0200)]
core/cgroup: fix return value of unit_cgorup_freezer_action()
We should return 0 only if current freezer state, as reported by the
kernel, is already the desired state. Otherwise, we would dispatch
return dbus message prematurely in bus_unit_method_freezer_generic().
Thanks to Frantisek Sumsal for reporting the issue.
Add %l as specifier for the hostname without any domain component
As described in #15603, it is a fairly common setup to use a fqdn as the
configured hostname. But it is often convenient to use just the actual
hostname, i.e. until the first dot. This adds support in tmpfiles, sysusers,
and unit files for %l which expands to that.
Add a basic test that the configured fallback hostname is OK
Ideally, assert_cc() would be used for this, so that it is not possible to even
compile systemd with something like '-Dfallback-hostname=.foo'. But to do a
proper check we need to call hostname_is_valid(), and we cannot depend on being
able to run code (e.g. during cross-compilation). So let's do a very superficial
check in meson, and a proper on in test-util.
This new helper checks whether the specified locale is installed. It's
distinct from locale_is_valid() which just superficially checks if a
string looks like something that could be a valid locale.
tmpfiles: optionally, read /etc/passwd + /etc/group without NSS
There are two libc APIs for accessing the user database: NSS/getpwuid(),
and fgetpwent(). if we run in --root= mode (i.e. "offline" mode), let's
use the latter. Otherwise the former. This means tmpfiles can use the
database included in the root environment for chowning, which is a lot
more appropriate.
man: put all pages which mention a specifier in the index
I wasn't 100% convinced that this is the right thing to do, hence the separate
commit. But e.g. for paths we index all mentions, so I think it's reasonable to
do the same here.
user-record: add new field for requesting LUKS discard on logout
We make this entirely independent of the regular discard field, i.e. the
one that controls discard behaviour when the home directory is online.
Not all combinations make a ridiculous amount of sense, but most do.
Specifically:
online-discard = yes, offline-discard = yes
→ Discard when activating explicitly, and during runtime using
the "discard" mount option, and discard explicitly when logging
out again.
online-discard = no, offline-discard = yes
→ The new default: when logging in allocate the full backing
store, and use no discard while active. When loging out discard
everything. This provides nice behaviour: we take minimal storage
when offline but provide allocation guarantees while online.
online-discard = no, offline-discard = no
→ Never, ever discard, always operate with fully allocated
backing store. The extra safe mode.
Let's make debugging a bit easier: when invoking homed from the build
tree it's now possible to make sure homed invokes the build tree's
homework binary by setting an env var.
The hack with getparent().txt is not very pretty, but the whole
thing seems to work well enough. It is useful to figure out whihc
specifiers are supported where.
tree-wide: make sure our control buffers are properly aligned
We always need to make them unions with a "struct cmsghdr" in them, so
that things properly aligned. Otherwise we might end up at an unaligned
address and the counting goes all wrong, possibly making the kernel
refuse our buffers.
Also, let's make sure we initialize the control buffers to zero when
sending, but leave them uninitialized when reading.
Both the alignment and the initialization thing is mentioned in the
cmsg(3) man page.
tree-wide: use CMSG_SPACE() (and not CMSG_LEN()) to allocate control buffers
We need to use the CMSG_SPACE() macro to size the control buffers, not
CMSG_LEN(). The former is rounded up to next alignment boundary, the
latter is not. The former should be used for allocations, the latter for
encoding how much of it is actually initialized. See cmsg(3) man page
for details about this.
Given how confusing this is, I guess we don't have to be too ashamed
here, in most cases we actually did get this right.
meson: rename make-man-rules.py to update-man-rules.py
The name of the helper didn't match the name of the meson target, which was
always confusing me. With this change, we consistenly use "update" to
re-generate things which we otherwise keep in vc, and "make" for things
which are generated during each build.
meson: drop "meson-" prefix from various helper script filenames
In a few cases, the prefix was originally necessary because a different helper
script was used for automake, and a different one for meson. But now we use
meson exclusively, and the prefix isn't useful. This also synchronizes the
target name, file name, and variable name in meson.build. The targets exposed
by meson didn't have the prefix, so the user interface is unchanged.
(The prefix is retained in the few tools that are used for meson itself,
e.g. meosn-vcs-tag.sh, meson-make-symlink.sh, etc.)
projects / thirdparty / systemd.git / log
