Dnia Niedziela, 25 Stycznia 2015 07:45 Andrei Borzenkov <arvidj...@gmail.com>
napisał(a)
> В Sun, 25 Jan 2015 03:37:09 +0100
> "Tomasz Pawlak" <toma...@wp.pl> пишет:
>
> > core/main.c:1519
> > /* Make sure we leave a core dump without panicing the
> > * kernel. */
> > if (getpid() == 1) {
> > install_crash_handler();
> >
> > r = mount_cgroup_controllers(arg_join_controllers);
> > if (r < 0)
> > goto finish;
> > }
> >
> > core/main.c:226
> > static void install_crash_handler(void) {
> > struct sigaction sa = {
> > .sa_handler = crash,
> > .sa_flags = SA_NODEFER,
> > };
> >
> > sigaction_many(&sa, SIGNALS_CRASH_HANDLER, -1);
> > }
> >
> > /shared/util.c:2207
> > int sigaction_many(const struct sigaction *sa, ...) {
> > va_list ap;
> > int r = 0, sig;
> >
> > va_start(ap, sa);
> > while ((sig = va_arg(ap, int)) > 0)
> > if (sigaction(sig, sa, NULL) < 0)
> > r = -errno;
> > va_end(ap);
> >
> > return r;
> > }
> >
> > shared/def.h:40
> > #define SIGNALS_CRASH_HANDLER SIGSEGV,SIGILL,SIGFPE,SIGBUS,SIGQUIT,SIGABRT
> >
> >
> > BUGS:
> > 1. Ignoring return value from sigaction_many(): all sig handlers can
> > silently fail to install, thus leaving the whole process unprotected
>
> What do you suggest to do in this case? We are running as PID1, we
> cannot just error exit.
>
You are right, but it's not as simple as it may look at first sight:
1. If we allow the process to continue without sig handlers installed, then
results can be just catastrophic: kernel panic with all the services launched
-> broken transanctions, half-written records/files, etc -> total mess,
corrupted or lost data etc.
So, since successfull installation of the sig handlers is one of the most
critical parts of initialisation, it is actualy safer to just quit. This is
just a critical fault (and is currently completely ignored).
2. Another thing is, that those signals are not equivalently important, f.e.
SIGABRT can be throwed by thousants lines of code in this project (by abort()),
so it is much more likely that assertion checking will prevent segfaults,
throwing SIGABRT instead. This means that SIGABRT is actually far more probable
than SIGSEGV.
This in turn leads to simple solution: the process should unconditionally exit
if hander for SIGABRT have failed to install, but with other sig handlers
failed, we may take a risk and continue.
In any case, such situation should be logged as soon as possible.
Ignoring this is just asking for catastrophe.
3. SIGFPE: how often the code uses FPU? -> I mean, that handler for this sig
can be dynamically installed/unistalled when needed, probably only on a thread
level, not for the whole process. This will allow to completely safely report
failed sigaction by assertion checking.
4. So, sigaction_many() should be removed (also because it is a vararg
function, what is rather bad idea), and a function for registering one sig
handler at a time should be used. Then, we can tell (log) which signals were
not registered by sigaction, and take conscious decision what to do next.
Regards.
> >
> > 2. SIGSEGV should be catched by a handler running on a separate stack
> > (SA_ONSTACK) - otherwise it can cause segfault itself, when the first
> > SIGSEGV which triggered the handler is caused by stack overflow.
> >
> > 3. SA_NODEFER makes no sense, since the handler is expected to catch only
> > first critical signal. With SA_NODEFER nesting of signals is possible, what
> > can cause unpredictable results, including uncatched stack overflow caused
> > by the handler itself.
> >
> > Regards.
> >
> >
> > _______________________________________________
> > systemd-devel mailing list
> > systemd-devel@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/systemd-devel
_______________________________________________
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
