[possible bug] missed wakeup in do_sigtimedwait()?

[possible bug] missed wakeup in do_sigtimedwait()?

[Al Viro]

do_sigtimedwait():
        spin_lock_irq(&tsk->sighand->siglock);
        sig = dequeue_signal(tsk, &mask, info);
nope, nothing posted yet
        if (!sig && timeout) {
                /*
                 * None ready, temporarily unblock those we're interested
                 * while we are sleeping in so that we'll be awakened when
                 * they arrive. Unblocking is always fine, we can avoid
                 * set_current_blocked().
                 */
                tsk->real_blocked = tsk->blocked;
                sigandsets(&tsk->blocked, &tsk->blocked, &mask);
                recalc_sigpending();
                spin_unlock_irq(&tsk->sighand->siglock);
... and now somebody sends us a signal.  signal_wake_up() does nothing,
since we are still in TASK_RUNNING at that point

                __set_current_state(TASK_INTERRUPTIBLE);
                ret = freezable_schedule_hrtimeout_range(to, tsk->timer_slack_ns,
                                                         HRTIMER_MODE_REL);
... and we go to sleep for the duration of timeout or until the next
signal to arrive.

                spin_lock_irq(&tsk->sighand->siglock);
                __set_task_blocked(tsk, &tsk->real_blocked);
                sigemptyset(&tsk->real_blocked);
                sig = dequeue_signal(tsk, &mask, info);
... now we finally dequeue the sucker that had been pending through the
entire timeout period.

        }
        spin_unlock_irq(&tsk->sighand->siglock);

Looks like that __set_current_state() should've been done before dropping
the siglock.  Am I missing something subtle here?  It's not a terribly
wide window, but it's not impossible to hit e.g. on KVM and it does look
like a missed wakeup problem...  For that matter, spin_unlock_irq() might
run irq handlers, so it's not impossible to hit on the real hardware either.

Re: [possible bug] missed wakeup in do_sigtimedwait()?

[Linus Torvalds]

On Sat, Sep 4, 2021 at 7:45 AM Al Viro <viro@zeniv.linux.org.uk> wrote:
>
> Looks like that __set_current_state() should've been done before dropping
> the siglock.  Am I missing something subtle here?

I agree, that seems like a bug, and your fix seems the trivially correct thing.

The bug goes back at least before ther bk history (in 2002).
Presumably since the introduction of the system call.

         Linus

Re: [possible bug] missed wakeup in do_sigtimedwait()?

[Linus Torvalds]

On Sat, Sep 4, 2021 at 9:59 AM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> I agree, that seems like a bug, and your fix seems the trivially correct thing.

Oh, never mind.  Signals are special.

Why?

Because TASK_INTERRUPTIBLE is special, and schedule() will check for
"am I trying to sleep while a signal is pending" and will never
actually sleep.

So you can't have missed wakeups from signals, because this sequence
is perfectly ok, by design:

 - signal comes in and is pending

 - we set TASK_INTERRUPTIBLE

 - we are thinking about something *entirely* different, like looking
at a pipe being emty

 - we schedule()

and the pending signal will just mean that we never go to sleep.

It's designed that way exactly so that people who have interruptible
sleeps don't need to think about signals at all - they can concentrate
on doing their own thing, and then do the "signal_pending()" check at
any point without caring.

This has always been true, I had just swapped out that logic from memory.

             Linus

Re: [possible bug] missed wakeup in do_sigtimedwait()?

[Al Viro]

On Sat, Sep 04, 2021 at 10:12:09AM -0700, Linus Torvalds wrote:
> On Sat, Sep 4, 2021 at 9:59 AM Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
> >
> > I agree, that seems like a bug, and your fix seems the trivially correct thing.
> 
> Oh, never mind.  Signals are special.
> 
> Why?
> 
> Because TASK_INTERRUPTIBLE is special, and schedule() will check for
> "am I trying to sleep while a signal is pending" and will never
> actually sleep.
> 
> So you can't have missed wakeups from signals, because this sequence
> is perfectly ok, by design:
> 
>  - signal comes in and is pending
> 
>  - we set TASK_INTERRUPTIBLE
> 
>  - we are thinking about something *entirely* different, like looking
> at a pipe being emty
> 
>  - we schedule()
> 
> and the pending signal will just mean that we never go to sleep.
> 
> It's designed that way exactly so that people who have interruptible
> sleeps don't need to think about signals at all - they can concentrate
> on doing their own thing, and then do the "signal_pending()" check at
> any point without caring.

Thanks.  AFAICS, it's this logics in __schedule():
                if (signal_pending_state(prev_state, prev)) {
                        WRITE_ONCE(prev->__state, TASK_RUNNING);
IOW, TASK_INTERRUPTIBLE with signal_pending() or TASK_WAKEKILL with
pending SIGKILL.  OK...

Re: [possible bug] missed wakeup in do_sigtimedwait()?

[Linus Torvalds]

On Sat, Sep 4, 2021 at 11:13 AM Al Viro <viro@zeniv.linux.org.uk> wrote:
>
> Thanks.  AFAICS, it's this logics in __schedule():
>                 if (signal_pending_state(prev_state, prev)) {
>                         WRITE_ONCE(prev->__state, TASK_RUNNING);

Exactly. As you note, it also handles the WAKEKILL case which is
basically equivalent (just for the "I can't take normal signals, but
I'm ok dying" case)

            Linus