Alan Jenkins [Wed, 28 Feb 2018 16:03:43 +0000 (16:03 +0000)]
core: don't freeze OnCalendar= timer units when the clock goes back a lot
E.g. if you have a monthly event and you set the computer clock back one
year, we can allow the next 12 monthly events to happen naturally. In fact
we already do this when you start a Persistent=yes timer, we just need to
apply the same logic when it's running and we notice the system clock
being set backwards.
Alan Jenkins [Wed, 28 Feb 2018 15:34:16 +0000 (15:34 +0000)]
core: let OnCalendar= timer units expire during suspend (#8231)
On timejumps, including suspend, timer_time_change() calls for a
re-calculation of the next elapse. Sadly I'm not quite sure what the
intended effect of this was! Because it was not managing to fire
OnCalendar= timers which fired during the suspend... unless the timer had
already fired once before.
Reported, entirely correctly as far as I can see, on stackexchange:
https://unix.stackexchange.com/questions/351829/systemd-timer-that-expired-while-suspended
/* If we know the last time this was
* triggered, schedule the job based relative
- * to that. If we don't just start from
- * now. */
+ * to that. If we don't, just start from
+ * the activation time. */
The same code is called for both the initial calculation and this
re-calculation. If we're _not_ already active, then this is before the
activation time has been recorded in the unit, so just use the current
time as before. The new code is mechanically adapted from the same
logic for `OnActiveSec=` (case TIMER_ACTIVE in the code which follows).
Tested with `date --set`.
Motivations:
* Rotate monitoring data from Atop into files which are named per-day.
Fedora currently implements this with a cron job that runs at midnight,
but that didn't handle suspend correctly either.
* unbound-anchor.timer on Fedora, is used to update DNSSEC "root trust
anchor" daily, before the TTL expires. It uses OnCalendar=daily
AccuracySec=24h. Which is a bit suspect because the TTL is 2 days, but I
think it has the right general idea.
None of the other timer settings are correct, because they would not
account for time spent in suspend. Unless you set WakeSystem
(this feature is currently undocumented).
* So in general, we can expect to see people using OnCalendar= for the same
cases as cron.daily and cron.monthly. Which use anacron to keep track of
jobs which should be run even if the system was down at the time.
Timers which are configured to run more frequently than that, are
unlikely to mind if they get run slightly more often that the writer
realized, relative to the amount of time the system was really running.
* From the user report above: "I only want to use remind to show a desktop
notification, it seems excessive to wake up the computer for that. Also,
I would like to get the reminder first thing in the morning, so the
OnActiveSec doesn't help with that."
Alan Jenkins [Wed, 28 Feb 2018 15:07:30 +0000 (15:07 +0000)]
core: timer_enter_waiting(): refactor `base` local variable
We have two variables `b` and `base`. `b` is declared within limited
scope; `base` is declared at the top of the function. However `base`
is actually only used within a scope which is exclusive of `b`. Clarify
by moving `base` inside the limited scope as well.
(Also `base` doesn't need initializing any more than `b` does. The
declaration of `base` is now immediately followed by a case analysis of
`v->base`, which serves almost exclusively to determine the value of
`base`).
Franck Bui [Wed, 28 Feb 2018 09:36:06 +0000 (10:36 +0100)]
rules: skip btrfs check if devices are not ready in 64-btrfs.rules (#8304)
If any devices are marked with 'SYSTEMD_READY=0' then we shouldn't run any
btrfs check on them.
Indeed there's no point in running "btrfs ready" on devices that already have
SYSTEMD_READY=0 set. Most probably such devices are members of a higher layer
aggregate device such as dm-multipath or software RAID. Doing IO on them wastes
time at best, and may cause delays, timeouts, or even hangs at worst (think
active-passive multipath or degraded RAID, for example).
It was initially reported at:
https://bugzilla.opensuse.org/show_bug.cgi?id=872929
kernel-install: Don't install BLS kernel images if dest dir doesn't exist (#8306)
The script shouldn't rely on a previous script exiting with a status code
that prevents it to be executed. Instead, should check if the destination
directory for the BLS kernel image exists and exit otherwise.
Let's include the command line to use to get the requested output. This
makes it easy to copy/paste the command line out, and add "--in-place"
to actually apply the changes "run-coccinelle.sh" outputs.
At various places we only want to close fds if they are not
stdin/stdout/stderr, i.e. fds 0, 1, 2. Let's add a unified helper call
for that, and port everything over.
rule-syntax-check: fix handling of runaway strings in comma splitting (#8298)
A runaway string should still be returned by the code that splits on
commas, so add a '?' to the regex so that the last '"?' in a string
still produces a valid block for the split code.
Tested:
ACTION=="remove\"GOTO=""
Which then produced:
$ test/rule-syntax-check.py src/login/70-uaccess.rules
# looking at src/login/70-uaccess.rules
Invalid line src/login/70-uaccess.rules:10: ACTION=="remove\"GOTO=""
clause: ACTION=="remove\"GOTO=""
seccomp: rework functions for parsing system call filters
This reworks system call filter parsing, and replaces a couple of "bool"
function arguments by a single flags parameter.
This shouldn't change behaviour, except for one case: when we
recursively call our parsing function on our own syscall list, then
we'll lower the log level to LOG_DEBUG from LOG_WARNING, because at that
point things are just a problem in our own code rather than in the user
configuration we are parsing, and we shouldn't hence generate confusing
warnings about syntax errors.
It doesn't work, spits out only rubbish and was already excluded of
run-coccinelle.sh. It's a pitty it doesn't work, but let's drop this
dead piece of code for now.
doc: add a new doc/ directory, and move two markdown docs into them
I figure sooneror later we'll have more of these docs, hence let's give
them a clean place to be.
This leaves NEWS and README/README.md as well as the LICENSE texts in
the root directory of the project since that appears to be customary for
Free Software projects.
Franck Bui [Fri, 23 Feb 2018 15:54:40 +0000 (16:54 +0100)]
rule-syntax-check: PROGRAM is not supposed to get value assigned
In udev man page, "PROGRAM" key is part of the keys which are used for
matching purposes so it should only be used with the compare operator "==".
Actually it doesn't really make sense to assign it a value.
udev code allows both "=" and "==" for PROGRAM and both are handled the same
way but for consistencies it's better to have only the compare operator allowed
by the rule syntax checker.
No rules shipped by systemd use PROGRAM key so nothing need to be changed in
our rule files.
gcc warns about unitialized memory access because it notices that ssize_t which
is < 0 could be cast to positive int value. We know that this can't really
happen because only -1 can be returned, but OTOH, in principle a large
*positive* value cannot be cast properly. This is unlikely too, since xattrs
cannot be too large, but it seems cleaner to just use a size_t to return the
value and avoid the cast altoghter. This makes the code simpler and gcc is
happy too.
The following warning goes away:
[113/1502] Compiling C object 'src/basic/basic@sta/xattr-util.c.o'.
In file included from ../src/basic/alloc-util.h:28:0,
from ../src/basic/xattr-util.c:30:
../src/basic/xattr-util.c: In function ‘fd_getcrtime_at’:
../src/basic/macro.h:207:60: warning: ‘b’ may be used uninitialized in this function [-Wmaybe-uninitialized]
UNIQ_T(A,aq) < UNIQ_T(B,bq) ? UNIQ_T(A,aq) : UNIQ_T(B,bq); \
^
../src/basic/xattr-util.c:155:19: note: ‘b’ was declared here
usec_t a, b;
^
Before
systemd-detect-virt[18498]: No virtualization found in DMI
systemd-detect-virt[18498]: No virtualization found in CPUID
systemd-detect-virt[18498]: Virtualization XEN not found, /proc/xen does not exist
systemd-detect-virt[18498]: This platform does not support /proc/device-tree
systemd-detect-virt[18498]: Failed to check for virtualization: No such file or directory
The first four lines are at debug level, so the user would only see that last
one usually, which is not very enlightening.
This now becomes:
systemd-detect-virt[21172]: No virtualization found in DMI
systemd-detect-virt[21172]: No virtualization found in CPUID
systemd-detect-virt[21172]: Virtualization XEN not found, /proc/xen does not exist
systemd-detect-virt[21172]: This platform does not support /proc/device-tree
systemd-detect-virt[21172]: /proc/cpuinfo not found, assuming no UML virtualization.
systemd-detect-virt[21172]: This platform does not support /proc/sysinfo
systemd-detect-virt[21172]: Found VM virtualization none
systemd-detect-virt[21172]: none
basic/log: add an assert that does not recurse into logging functions
Then it can be used in the asserts in logging functions without causing
infinite recursion. The error is just printed to stderr, it should be
good enough for the common case.
gcc-8 throws an error if it knows snprintf might truncate output and the
return value is ignored:
../src/udev/udev-builtin-net_id.c: In function 'dev_pci_slot':
../src/udev/udev-builtin-net_id.c:297:47: error: '%s' directive output may be truncated writing up to 255 bytes into a region of size between 0 and 4095 [-Werror=format-truncation=]
snprintf(str, sizeof str, "%s/%s/address", slots, dent->d_name);
^~
../src/udev/udev-builtin-net_id.c:297:17: note: 'snprintf' output between 10 and 4360 bytes into a destination of size 4096
snprintf(str, sizeof str, "%s/%s/address", slots, dent->d_name);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
cc1: some warnings being treated as errors
Let's check all return values. This actually makes the code better, because there's
no point in trying to open a file when the name has been truncated, etc.
basic/log: make sure header is printed correctly, add test
If log_do_header() was called with overly long parameters, it'd generate
improper output. Essentially, it'd be truncated at random point, in particular
missing a newline at the end, so it'd run with the next field, usually MESSAGE=.
log_do_header is called with parameters from compiled code (file name, lien
nubmer, etc), so in practice this was unlikely to ever be a problem, but it is
possible. In particular, if systemd was compiled from sources in some deeply
nested directory (which happens for example in mock and other build roots), the
filename could be very long.
As a safety measure, let's truncate all parameters to 256 bytes. So we have
5 fields which are 256 bytes (plus the field name prefix), and a few other
fields with fixed width. This must always fit in the 2048 byte buffer.
I don't think there's much gain in calculating the required length precisely,
since it's a lot of fields and a few bytes allocated on the stack don't matter.
basic/log: fix confusion with parameters to log_dispatch_internal
log_dispatch_internal has only one caller where the extra_field/extra
params are not null: log_unit_full. When log_unit_full() was called,
when we got to log_dispatch_internal, our header would look like this:
PRIORITY=7
SYSLOG_FACILITY=3
CODE_FILE=../src/core/manager.c
CODE_LINE=2145
CODE_FUNC=manager_invoke_sigchld_event
USER_UNIT=gnome-terminal-server.service
65dffa7a3b984a6d9a46f0b8fb57710bUSER_INVOCATION_ID=
SYSLOG_IDENTIFIER=systemd
It took me a while to understand why I'm not seeing mangled messages in the
journal (after all, "" is a valid rvalue for log messages). The answer is that
journald rejects any field name which starts with a digit, and the MESSAGE_ID
that was used here starts with a digit. Hence, those lines would be silently
filtered out.
Peter Hutterer [Fri, 23 Feb 2018 08:36:45 +0000 (18:36 +1000)]
udev: don't assign INPUT_ID_MOUSE to a touchpad/joystick/touchscreen (#8259)
If a touchpad has MT axes only but not ABS_X/ABS_Y (DualShock 4 controller),
then we hit both the conditions is_touchpad and the later check for
!has_abs_axes here, assigning is_mouse and ID_INPUT_MOUSE later.
This is a bug, we historically only assigned either of of the pointing device
tags ID_INPUT_MOUSE/TOUCHPAD/JOYSTICK/TOUCHSCREEN, never multiple of them.
Note that we cannot just check for has_abs_axes and has_mt_coordinates because
the apple touch mouse has both. We really need to check if the device has
already been assigned something else.
Alan Jenkins [Thu, 22 Feb 2018 20:38:44 +0000 (20:38 +0000)]
login: fix user@.service case, so we don't allow nested sessions (#8051)
> logind sessions are mostly bound to the audit session concept, and audit
> sessions remain unaffected by "su", in fact they are defined to be
> "sealed off", i.e. in a way that if a process entered a session once, it
> will always stay with it, and so will its children, i.e. the only way to
> get a new session is by forking off something off PID 1 (or something
> similar) that never has been part of a session.
The code had a gap. user@.service is a special case PAM session which does
not create a logind session. Let's remember to check for it.
Fix format-truncation compile failure by typecasting USB IDs (#8250)
This patch adds safe_atoux16 for parsing an unsigned hexadecimal 16bit int, and
uses that for parsing USB device and vendor IDs.
This fixes a compile error with gcc-8 because while we know that USB IDs are 2 bytes,
the compiler does not know that.
../src/udev/udev-builtin-hwdb.c:80:38: error: '%04X' directive output may be
truncated writing between 4 and 8 bytes into a region of size between 2 and 6
[-Werror=format-truncation=]
Signed-off-by: Adam Williamson <awilliam@redhat.com> Signed-off-by: Patrick Uiterwijk <puiterwijk@redhat.com>
libsystemd-network: fix endianness in ARP BPF filter (#8255)
Commit f11cba7479fe ("libsystemd-network: fix unaligned loads (issue #7654)")
changed the way in which the MAC address is read to use native endiannes:
This is wrong because loads done with BPF_LD + BPF_ABS are big-endian, as it
can be seen for the ethertype and arp-operation loads above in the
filter. Also, the same commit changed:
htobe32(*((unsigned int *)x) -> unaligned_read_be32(x)
in _bind_raw_socket(), which is the correct form.
The commit broke IPv4LL in presence of loops, as the sender now considers its
own packets as conflicting.
systemctl,man: use UNIT as the placeholder for a unit name
NAME is kind of meaningless, because everything has a name. "Unit"
makes it more obvious that a name of a unit is necessary. I was always
momentarily baffled by "set-property NAME ASSIGNMENT...", where there
are two objects (the unit and the property), and it's not clear which of
the two "NAME" is supposed to signify.
Sergey Ptashnick [Thu, 22 Feb 2018 13:25:11 +0000 (16:25 +0300)]
Update Russian translation (#8248)
Used "in"-form here (i.e. "зарегистрировать службу *в* DNS-SD") because
simply "служба DNS-SD" may be confused with resolved itself (at least in
Russian).
nologin: extend the /run/nologin descriptions a bit (#8244)
This is an attempt to improve #8228 a bit, by extending the /run/nologin
a bit, but still keeping it somewhat brief.
On purpose I used the vague wording "unprivileged user" rather than
"non-root user" so that pam_nologin can be updated to disable its
behaviour for members of the "wheel" group one day, and our messages
would still make sense.
When the user is dynamic, and we are setting up state, cache, or logs dirs,
behaviour is unchanged, we always do a recursive chown. This is necessary
because the user number might change between invocations.
But when setting up a directory for non-dynamic user, or a runtime directory
for a dynamic user, do any ownership or mode changes only when the directory
is initially created. Nothing says that the files under those directories have
to be all recursively owned by our user. This restores behaviour before 3536f49e8fa281539798a7bc5004d73302f39673, so modifications to the state of
the runtime directory persist between ExecStartPre's and ExecStart's, and even
longer in case the directory is persistent.
I think it _would_ be a nice property if setting a user would automatically
propagate to ownership of any Runtime/Logs/Cache directories. But this is
incompatible with another nice property, namely preserving changes to those
directories made by an admin, and with allowing change of ownership of files
in those directories by the service (e.g. to allow other users to access them).
Of the two, I think the second property is more important. Also, it's backwards
compatible.
There is no need to chmod a directory we just created, so move that step
up into a branch. After that, 'effective' is only used once, so get rid of
it too.
Generally we prefer 'return' from main() over exit() so that automatic
cleanups and such work correct. Let's do that in shutdown.c too, becuase
there's not really any reason not to.
With this we are pretty good in consistently using return from main()
rather than exit() all across the codebase. Yay!
basic: split out update_reboot_parameter_and_warn() into its own .c/.h files
This is primarily preparation for a follow-up commit that adds a common
implementation of the other side of the reboot parameter file, i.e. the
code that reads the file and issues reboot() for it.
basic: add a common syscall wrapper around reboot()
This mimics the raw_clone() call we have in place already and
establishes a new syscall wrapper raw_reboot() that wraps the kernel's
reboot() system call in a bit more low-level fashion that glibc's
reboot() wrapper. The main difference is that the extra "arg" argument
is supported.
Ultimately this just replaces the syscall wrapper implementation we
currently have at three places in our codebase by a single one.
With this change this means that all our syscall() invocations are
neatly separated out in static inline system call wrappers in our header
functions.
projects / thirdparty / systemd.git / log
