Behaviour is not identical, as shown by the tests in test-strv.
The combination of EXTRACT_UNQUOTE without EXTRACT_RELAX only appears in
the test, so it doesn't seem particularly important. OTOH, the difference
in handling of squished parameters could make a difference. New behaviour
is what both bash and python do, so I think we can ignore this corner case.
This change has the following advantages:
- the duplication of code paths that do a very similar thing is removed
- extract_one_word() / strv_split_extract() return a proper error code.
This tries to address the "bind"/"unbind" uevent kernel API breakage, by
changing the semantics of device tags.
Previously, tags would be applied on uevents (and the database entries
they result in) only depending on the immediate context. This means that
if one uevent causes the tag to be set and the next to be unset, this
would immediately effect what apps would see and the database entries
would contain each time. This is problematic however, as tags are a
filtering concept, and if tags vanish then clients won't hence notice
when a device stops being relevant to them since not only the tags
disappear but immediately also the uevents for it are filtered including
the one necessary for the app to notice that the device lost its tag and
hence relevance.
With this change tags become "sticky". If a tag is applied is once
applied to a device it will stay in place forever, until the device is
removed. Tags can never be removed again. This means that an app
watching a specific set of devices by filtering for a tag is guaranteed
to not only see the events where the tag is set but also all follow-up
events where the tags might be removed again.
This change of behaviour is unfortunate, but is required due to the
kernel introducing new "bind" and "unbind" uevents that generally have
the effect that tags and properties disappear and apps hence don't
notice when a device looses relevance to it. "bind"/"unbind" events were
introduced in kernel 4.12, and are now used in more and more subsystems.
The introduction broke userspace widely, and this commit is an attempt
to provide a way for apps to deal with it.
While tags are now "sticky" a new automatic device property
CURRENT_TAGS is introduced (matching the existing TAGS property) that
always reflects the precise set of tags applied on the most recent
events. Thus, when subscribing to devices through tags, all devices that
ever had the tag put on them will be be seen, and by CURRENT_TAGS it may
be checked whether the device right at the moment matches the tag
requirements.
See: #7587#7018#8221
When running the program with udev_event_spawn it is possible to miss
output in stdout when the program exits causing the result to be empty
which can cause rules using the result to not function correctly.
This is due to the on_spawn_sigchld callback being processed while IO is
still pending and causing the event loop to exit.
To correct this the sigchld event source is made a lower priority than
the other event sources to ensure it is processed after IO. This
requires changing the IO event source to oneshot and re-enabling it when
valid data is read but not for EOF, this prevents the empty pipes
constantly generating IO events.
let's add [static] where it was missing so far
Drop [static] on parameters that can be NULL.
Add an assert() around parameters that have [static] and can't be NULL
hence.
Add some "const" where it was forgotten.
This partially reverts 25de7aa7b9. I don't think the
change was intended there.
The problem I'm trying to solve: for /dev/kvm we get first an ADD uevent, and
then CHANGE whenever something connects or disconnects to the character device.
The rules in 50-default-udev.rules set UID, GID, and MODE on ADD, but not on
CHANGE. When the change event happens, we would reset the ownership and
permissions.
This happens because node_permissions_apply() would (after 25de7aa7b9)
set uid=gid=0 if they weren't set by the rules.
So let's only pass uid/gid/mode to node_permissions_apply() if appropriately
configured. Also let node_permissions_apply() do the skip of uid/gid/mode if
not set, and rename "always_apply" to more closely reflect its meaning.
It is pretty hard to figure out what the problem actually is, esp. when the rule
is long.
On my machine:
systemd[1]: Starting udev Kernel Device Manager...
systemd-udevd[217399]: /usr/lib/udev/rules.d/11-dm-lvm.rules:40 Invalid value for OPTIONS key, ignoring: 'event_timeout=180'
systemd-udevd[217399]: /usr/lib/udev/rules.d/11-dm-lvm.rules:40 The line takes no effect, ignoring.
systemd-udevd[217399]: /etc/udev/rules.d/60-ipath.rules:4 Invalid value "kcopy/%02n" for NAME (char 7: invalid substitution type), ignoring, but please fix it.
systemd-udevd[217399]: /usr/lib/udev/rules.d/65-md-incremental.rules:28 Invalid value "/sbin/mdadm -I $env{DEVNAME} --export $devnode --offroot ${DEVLINKS}" for IMPORT (char 58: invalid substitution type), ignoring, but please fix it.
systemd-udevd[217399]: /etc/udev/rules.d/73-special-net-names.rules:14 Invalid value "/bin/sh -ec 'D=${DEVPATH#*/vio/}; D=${D%%%%/*}; D=${D#????}; D=${D#0}; D=${D#0}; D=${D#0}; D=${D#0}; echo ${D:-0}'" for PROGRAM (char 16: invalid substitution type), ignoring, but please fix it.
systemd-udevd[217399]: /usr/lib/udev/rules.d/84-nm-drivers.rules:10 Invalid value "/bin/sh -c 'ethtool -i $1 | sed -n s/^driver:\ //p' -- $env{INTERFACE}" for PROGRAM (char 24: invalid substitution type), ignoring, but please fix it.
systemd-udevd[217399]: /usr/lib/udev/rules.d/90-libgpod.rules:19 IMPORT key takes '==' or '!=' operator, assuming '==', but please fix it.
systemd-udevd[217399]: /usr/lib/udev/rules.d/90-libgpod.rules:23 IMPORT key takes '==' or '!=' operator, assuming '==', but please fix it.
systemd-udevd[217399]: /usr/lib/udev/rules.d/99-vmware-scsi-udev.rules:5 Invalid value "/bin/sh -c 'echo 180 >/sys$DEVPATH/device/timeout'" for RUN (char 27: invalid substitution type), ignoring, but please fix it.
systemd-udevd[217399]: /usr/lib/udev/rules.d/99-vmware-scsi-udev.rules:6 Invalid value "/bin/sh -c 'echo 180 >/sys$DEVPATH/device/timeout'" for RUN (char 27: invalid substitution type), ignoring, but please fix it.
systemd[1]: Started udev Kernel Device Manager.
This does the following:
- rename enum udev_builtin_cmd -> UdevBuiltinCmd
- rename struct udev_builtin -> UdevBuiltin
- move type definitions to udev-rules.h
- move prototypes of functions defined in udev-rules.c to udev-rules.h
- drop to use strbuf
- propagate critical errors in applying rules,
- drop limitation for number of tokens per line.
Fixes the following build failure with musl:
../git/src/udev/udev-event.c: In function 'spawn_wait':
../git/src/udev/udev-event.c:600:53: error: 'WEXITED' undeclared (first use in this function); did you mean 'WIFEXITED'?
r = sd_event_add_child(e, NULL, spawn->pid, WEXITED, on_spawn_sigchld, spawn);
^~~~~~~
This looks like a bug in udev-event.c that could also have broken
the compilation after some future glibc header reshuffle.
This fixes bugs introduced by 29448498c7
and d838e14515.
Previously, RUN and SECLABEL keys are stored in udev_list with its unique
flag is false. If the flag is false, then udev_list is just a linked
list and new entries are always added in the last.
So, we should use OrderedHashmap instead of Hashmap.
Fixes#11368.
When running PROGRAM="...", we would log
systemd-udevd[447]: Failed to wait spawned command '...': Input/output error
no matter why the program actually failed, at error level.
The code wouldn't distinguish between an internal failure and a failure in the
program being called and run sd_event_exit(..., -EIO) on any kind of error. EIO
is rather misleading here, becuase it suggests a serious error.
on_spawn_sigchld is updated to set the return code to distinguish failure to
spawn, including the program being killed by a signal (a negative return value),
and the program failing (positive return value).
The logging levels are adjusted, so that for PROGRAM= calls, which are
essentially "if" statements, we only log at debug level (unless we get a
timeout or segfault or another unexpected error).
We had two very similar functions: device_read_db_aux and device_read_db,
and a number of wrappers for them:
device_read_db_aux
← device_read_db (in sd-device.c)
← all functions in sd-device.c, including sd_device_is_initialized
← device_read_db_force
← event_execute_rules_on_remove (in udev-event.c)
device_read_db (in device-private.c)
← functions in device_private.c (but not device_read_db_force):
device_get_devnode_{mode,uid,gid}
device_get_devlink_priority
device_get_watch_handle
device_clone_with_db
← called from udevadm, udev-{node,event,watch}.c
Before 7141e4f62c (sd-device: don't retry loading
uevent/db files more than once), the two implementations were the same. In that
commit, device_read_db_aux was changed. Those changes were reverted in the parent
commit, so the two implementations are now again the same except for superficial
differences. This commit removes device_read_db (in sd-device.c), and renames
device_read_db_aux to device_read_db_internal and makes everyone use this one
implementation. There should be no functional change.
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.
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.