2001-10-11 23:12:53

by Paul Menage

[permalink] [raw]
Subject: Race conditions accessing sig->action?


The task->sig->siglock spinlock is only used in do_sigaction(), and not
in the arch-specific signal dispatch code nor the signal copying code
in fs/exec.c and kernel/fork.c. While this is fine if the signal
structures aren't shared between processes, if they are then it seems as
though some race conditions can creep in, e.g.

- one thread changes the SIGCHLD handler from a handler to SIG_IGN just
as a different thread on a different CPU receives a SIGCHLD. The second
thread can end up segfaulting, as by the time we get to setting up the
signal frame, sa_handler for SIGCHLD has changed from a valid pointer to
SIG_IGN.

- one thread fork()s just as another thread changes the signal handler
for a signal, and copy_sighand() catches the handler structure in an
inconsistent state.

These are pretty unlikely races, and it could be regarded as an error
for user-space to change a shared signal handler anyway. If that's the
case, shouldn't we return an error from sigaction() if the sig->count
structure is not 1.

If changing shared signal handlers is really required (e.g. for
pthreads?) then - unless I'm missing something fundamental -
copy_sighand() and the signal dispatch code ought to take siglock. The
dispatch code would probably need to copy the relevant parts of the
signal action within the spinlock, and use the copy, since setting up
the signal frame can block on page faults.

On a separate note, there seems to be a lot of duplication of code in
the various arch-specific signal dispatch code. Would there be
objections to a patch to bring some of this functionality (for those
architectures that are fairly similar) back into the arch-independent
signal code?

Paul