| Age | Commit message (Collapse) | Author |
|---|
|
Leftover from 6af5e6a4c918a68b196a04346732e094e5373a36
|
|
This is essentially a revert of 5c67cf2 and fixes issue #190.
|
|
Make sure we never close fds before we drop their related event-source.
This will cause horrible disruptions if the fd-num is re-used by someone
else. Under normal conditions, this should not cause any problems as the
close() will drop the fd from the epoll-set automatically. However, this
changes if you have any child processes with a copy of that fd.
This fixes issue #163.
Background:
If you create an epoll-set via epoll_create() (lets call it 'EFD')
you can add file-descriptors to it to watch for events. Whenever
you call EPOLL_CTL_ADD on a file-descriptor you want to watch, the
kernel looks up the attached "struct file" pointer, that this FD
refers to. This combination of the FD-number and the "struct file"
pointer is used as key to link it into the epoll-set (EFD).
This means, if you duplicate your file-descriptor, you can watch
this file-descriptor, too (because the duplicate will have a
different FD-number, hence, the combination of FD-number and
"struct file" is different as before).
If you want to stop watching an FD, you use EPOLL_CTL_DEL and pass
the FD to the kernel. The kernel again looks up your
file-descriptor in your FD-table to find the linked "struct file".
This FD-number and "struct file" combination is then dropped from
the epoll-set (EFD).
Last, but not least: If you close a file-descriptor that is linked
to an epoll-set, the kernel does *NOTHING* regarding the
epoll-set. This is a vital observation! Because this means, your
epoll_wait() calls will still return the metadata you used to
watch/subscribe your file-descriptor to events.
There is one exception to this rule: If the file-descriptor that
you just close()ed was the last FD that referred to the underlying
"struct file", then _all_ epoll-set watches/subscriptions are
destroyed. Hence, if you never dup()ed your FD, then a simple
close() will also unsubscribe it from any epoll-set.
With this in mind, lets look at fork():
Assume you have an epoll-set (EFD) and a bunch of FDs
subscribed to events on that EFD. If you now call fork(),
the new process gets a copy of your file-descriptor table.
This means, the whole table is copied and the "struct
file" reference of each FD is increased by 1. It is
important to notice that the FD-numbers in the child are
exactly the same as in the parent (eg., FD #5 in the child
refers to the same "struct file" as FD #5 in the parent).
This means, if the child calls EPOLL_CTL_DEL on an FD, the
kernel will look up the linked "struct file" and drop the
FD-number and "struct file" combination from the epoll-set
(EFD). However, this will effectively drop the
subscription that was installed by the parent.
To sum up: even though the child gets a duplicate of the
EFD and all FDs, the subscriptions in the EFD are *NOT*
duplicated!
Now, with this in mind, lets look at what udevd does:
Udevd has a bunch of file-descriptors that it watches in its
sd-event main-loop. Whenever a uevent is received, the event is
dispatched on its workers. If no suitable worker is present, a new
worker is fork()ed to handle the event. Inside of this worker, we
try to free all resources we inherited. However, the fork() call
is done from a call-stack that is never rewinded. Therefore, this
call stack might own references that it drops once it is left.
Those references we cannot deduce from the fork()'ed process;
effectively causing us to leak objects in the worker (eg., the
call to sd_event_dispatch() that dispatched our uevent owns a
reference to the sd_event object it used; and drops it again once
the function is left).
(Another example is udev_monitor_ref() for each 'worker' that is
also inherited by all children; thus keeping the udev-monitor and
the uevent-fd alive in all children (which is the real cause for
bug #163))
(The extreme variant is sd_event_source_unref(), which explicitly
keeps event-sources alive, if they're currently dispatched,
knowing that the dispatcher will free the event once done. But
if the dispatcher is in the parent, the child will never ever
free that object, thus leaking it)
This is usually not an issue. However, if such an object has a
file-descriptor embedded, this FD is left open and never closed in
the child.
In manager_exit(), if we now destroy an object (i.e., close its embedded
file-descriptor) before we destroy its related sd_event_source, then
sd-event will not be able to drop the FD from the epoll-set (EFD). This
is, because the FD is no longer valid at the time we call EPOLL_CTL_DEL.
Hence, the kernel cannot figure out the linked "struct file" and thus
cannot remove the FD-number plus "struct file" combination; effectively
leaving the subscription in the epoll-set.
Since we leak the uevent-fd in the children, they retain a copy of the FD
pointing to the same "struct file". Thus, the EFD-subscription are not
automatically removed by close() (as described above). Therefore, the main
daemon will still get its metadata back on epoll_watch() whenever an event
occurs (even though it already freed the metadata). This then causes the
free-after-use bug described in #163.
This patch fixes the order in which we destruct objects and related
sd-event-sources. Some open questions remain:
* Why does source_io_unregister() not warn on EPOLL_CTL_DEL failures?
This really needs to be turned into an assert_return().
* udevd really should not leak file-descriptors into its children. Fixing
this would *not* have prevented this bug, though (since the child-setup
is still async).
It's non-trivial to fix this, though. The stack-context of the caller
cannot be rewinded, so we cannot figure out temporary refs. Maybe it's
time to exec() the udev-workers?
* Why does the kernel not copy FD-subscriptions across fork()?
Or at least drop subscriptions if you close() your FD (it uses the
FD-number as key, so it better subscribe to it)?
Or it better used
FD+"struct file_table*"+"struct file*"
as key to not allow the childen to share the subscription table..
*sigh*
Seems like we have to live with that API forever.
|
|
This ports a lot of manual code over to sigprocmask_many() and friends.
Also, we now consistly check for sigprocmask() failures with
assert_se(), since the call cannot realistically fail unless there's a
programming error.
Also encloses a few sd_event_add_signal() calls with (void) when we
ignore the return values for it knowingly.
|
|
udevd: event - don't log about failures of spawn processes when this …
|
|
|
|
It's only marginally shorter then the usual for() loop, but certainly
more readable.
|
|
PROGRAM and IMPORT{program} uses the exit code of the spawn process to decide if a rule matches or not,
a failing process is hence normal operation and not something we should warn about.
We still warn about other types of failing processes.
|
|
|
|
The documentation claims hwdb entries may be placed in the volatile
/run/udev/hwdb.d directory but nothing actually looked at it.
|
|
|
|
Now that listen_fds() have been split out, we can safely move the allocation
of the manager object after doing the forking (the fork is done to notify legcay
init-systems that the fds are ready).
Subsequently, we can merge manager_listen() back into managre_new().
This entails a minor behaviour change: the application of permissions to
static device nodes now happens after the fork (but still before notifying
systemd about being ready).
|
|
This will simply silently fail on non-systemd systems, so there is no reason
to make it conditional.
Also make it clear that we notify systemd about being ready as the last step
before starting the event loop, whereas the forking might need to happen
earlier.
|
|
This will allow us in a follow-up commit to listen to fds before forking and
still allocate the manager only after the fork.
|
|
Hide the differenec in listen_fds, by simply opening the fds
here in case they are not passed in.
|
|
This should have no behavioural change, but it is odd to tie the cgroup cleaning to
whether or not we are passed sockets.
The point really is if we are guaranteed to be in a dedicated cgroup, so instead
check for our parent being PID1 (we already implicitly only do this on systemd
systems).
|
|
If they are passed from PID1 this is not necessary.
|
|
|
|
We used to block all signals, and restore the original signal mask before exec'ing
external processes.
Now we just block the signals we care about and unconditionally unblock all signals
before exec'ing.
|
|
Peek at the ABS_MT_SLOT-1 axis. Expect that touch screens only
have axes inside the MT range.
|
|
A lot of touch screens use INPUT_PROP_DIRECT to indicate that touch input
maps directly to the underlying screen, while the BTN_TOUCH bit might not be
set.
|
|
This change switches to bools and separates bit flag evaluation from
decision making and application of udev properties, while hopefully
keeping the same semantics. Apart from using BTN_LEFT instead of BTN_MOUSE
for mouse detection.
|
|
Being explicit about this makes the code easier to follow IMHO.
|
|
Mostly for documentation purposes.
|
|
The communication channels must all be opened before forknig in daemon mode,
or we cannot guarantee that udevadm will work correctly as soon as udevd is
started.
|
|
In daemon mode we would break sd-event as it cannot work accross different processes.
Simply delay the allocation to after the fork.
|
|
Kay said: 'it is from ancient times, when we started udevd from the
kernel's usermodhelper which had no fd 0,1,2'.
|
|
No functional changes.
|
|
This notifies PID1 about config being flushed, about shutdown starting and shutdown finalizing.
|
|
Only unset the env var in the workers, but otherwise keep it around in the main daemon.
|
|
Only log about starting in daemon mode, rely on PID1 to log this in notify mode. Also
explicitly set the STATUS variable, as is done in notify mode as is done for other
serivecs.
|
|
This allows us to drop the special sigterm handling in spawn_wait()
as this will now be passed directly to the worker event loop.
We now log failing spawend processes at 'warning' level, and timeouts
are in terms of CLOCK_BOOTTIME when available, otherwise the behavior
is unchanged.
|
|
|
|
Rather than trying to schedule new events on every main-loop iteration, do it explicitly when
processing an event finishes, a worker is killed, a new uevent is received, or the event queue
is explicitly restarted.
|
|
Also move builtin and rules initialization from main loop to
event_queue_start().
No functional change.
|
|
The behavior is mostly unchanged, but rather than only ever calling these functions at
fixed points in the event loop, they are called directly whenever they are invoked.
|
|
We were listening for SIGCHLD in the wrong process.
|
|
This partly reverts:
commit 6d1b1e0bc6bd020218afc5f05286bf372be283d5
Author: Tom Gundersen <teg@jklm.no>
Date: Sun May 24 15:10:04 2015 +0200
udevd: worker - fully clean up unnecessary fds
The inotify-fd _is_ used in the workers, so don't close it! Have a look at
udev-watch.c, which keeps track of the inotify-fd as a global variable
(ugh!).
|
|
We would enforce that events could only be added to the queue from the
main process, but that brake in daemonized mode. Relax the restriction
to only allow one process to add events to the queue.
Reported by Mantas Mikulėnas.
|
|
Initialize structs when declaring rather than using memzero().
|
|
These are only ever used in the parent process, so close them early in the worker.
|
|
The original code used fread(), which on some libc implementions
(ie glibc 2.17) would pre-read a full 4K (PAGE_SIZE) of the
PCI config space, when only 64 bytes were requested.
I have recently come across PCIe hardware which responds with
Completion Timeouts when accesses above 256 bytes are attempted.
This can cause server systems with GHES/AEPI support to cause
and immediate kernel panic due to the failed PCI transaction.
This change replaces the buffered fread() with an explict
unbuffered read() of 64 bytes, which corrects this issue by
only reading the guaranteed first 64 bytes of PCIe config space.
|
|
|
|
The parser used for MTU and Speed expects them to be size_t, not unsigned int.
This caused a corruption in the rest of the structure.
Reported by David O Neill <david.m.oneill@intel.com>.
|
|
Makes it a bit clearer what is going on, rather than jumping to the end of main().
No functional change.
|
|
First parse config, then sanitize environment before donig any further setup.
No functional change.
|
|
No functional change.
|
|
This uses kill_and_sigcont() instead of kill(), otherwise no functional change.
|
|
We were returning rather than continuing in some cases. The intention
was always to fully process all pending events before returning
from the SIGCHLD handler. Restore this behaviour.
|
|
No functional change.
|
