| Summary: | [timer_create / SIGEV_THREAD] signalmask of timer_sigev_thread dangerous | ||
|---|---|---|---|
| Product: | glibc | Reporter: | Roland Lezuo <roland.lezuo> |
| Component: | librt | Assignee: | Adhemerval Zanella <adhemerval.zanella> |
| Status: | RESOLVED FIXED | ||
| Severity: | normal | CC: | adhemerval.zanella, bugdal, glibc-bugs, tpiepho |
| Priority: | P2 | Flags: | fweimer:
security-
|
| Version: | 2.9 | ||
| Target Milestone: | 2.32 | ||
| Host: | Target: | ||
| Build: | Last reconfirmed: | ||
The behavior is correct. You're relying on undefined behavior. Just don't do it. (In reply to comment #1) > The behavior is correct. You're relying on undefined behavior. Just don't do > it. This means it's impossible to use sigwait()/sigwaitinfo()/sigtimedwait() in the same process with a POSIX timer using SIGEV_THREAD. It would be much more useful if the helper thread had the same signal mask as the thread that originally called timer_create. That signal mask is already retrieved when the timer thread is created. If it were simply stored somewhere, then the helper thread could set its mask to that instead of an empty set. Drepper's comment is technically incorrect; the behavior is implementation-defined, not undefined. And in this case, glibc is actively providing detrimental behavior. I think this should be fixed. As Trent suggested, it may be convenient to have the signal mask match the mask at the time of the call to timer_create, but storing this and restoring it would be extra work. I think the most-correct behavior is for the timer thread always to start with all signals blocked. If the applications wants some signals unblocked, it can always unblock them as part of the timer handling code. But if any of them start out unblocked, it's impossible to block them without race conditions. Basically, this just comes down to common-sense about signal masking and race conditions. A library function (whether in the standard library or a third-party library) should NEVER unblock a signal unless that signal is only used for its own internal purposes (like the timer or cancellation signals). The only operations libraries ever perform on the signal mask should be blocking signals and restoring to a previously-saved mask. The signal mask is already stored by code in the timer_create() call, so using that mask in the timer thread doesn't actually take any more work that is done now. It's just as matter of changing two existing rt_sigprocmask calls to use a global rather than a local variable. But thinking about it more, I agree with Rich that blocking all signals is the only sensible way. The sigprocmask call in the timer thread can be removed. The user code can always add it back and unblock whatever it wants unblocked if it cares. If it doesn't, then the call is avoided. Restoring the mask used when calling timer_create() wouldn't work if the process has since blocked a signal. For instance, if it needs to modify a data structure also used by a signal handler, it's necessary to block the signal while doing so to avoid racing with the handler. If a completely unrelated timer goes off and creates a new thread with unblocked signals.... Using a global is definitely not viable; the sigset_t would have to be added to the structure allocated for the particular timer. There's no reason this couldn't be done, but I think we're both in agreement that starting SIGEV_TIMER threads with all signals blocked is the most desirable (and most flexible) behavior anyway. Fixed on 2.32. |
When a POSIX timer is created with timer_create and sigev_notify = SIGEV_THREAD glibc uses internal helper threads (timer_sigev_thread) to invoke the callback function. This helper threads unblocks all signals. <snip> /* Helper thread to call the user-provided function. */ static void * timer_sigev_thread (void *arg) { /* The parent thread has all signals blocked. This is a bit surprising for user code, although valid. We unblock all signals. */ sigset_t ss; sigemptyset (&ss); INTERNAL_SYSCALL_DECL (err); INTERNAL_SYSCALL (rt_sigprocmask, err, 4, SIG_SETMASK, &ss, NULL, _NSIG / 8); struct thread_start_data *td = (struct thread_start_data *) arg; <snap> According to the POSIX standard the signalmask of the thread executing the timer callback is implementation defined. The above comments indicates that the signal mask is cleared to not suprise user code. I have an application which uses multiple rt-signals. One of them is blocked by all threads and received using sigwaitinfo. The behaviour of timer_sigev_thread opens a race condition where the rt-signal is delivered and because there is no signal handler the application crashes. It is unfeasible to add a signal handler and propagate the event back to the signal handling code. I did not actually try on FreeBSD, but it seems they do not clear the signalmask. I think the correct way is to assume worst and have the signalmask blocking all signals. User code must be prepared to be executed with any signalmask, and by doing so glibc does not crash user applications depending on blocked signals and sigwaitinfo.