RE: [RFC PATCH 1/5] signal: Teach sigsuspend to use set_user_sigmask

[David Laight <David.Laight@ACULAB.COM>]

From: Eric W. Biederman
> Sent: 10 June 2019 22:21
...
> >
> > As for "remove saved_sigmask" I have some concerns... At least this
> > means a user-visible change iiuc. Say, pselect unblocks a fatal signal.
> > Say, SIGINT without a handler. Suppose SIGINT comes after set_sigmask().
> >
> > Before this change the process will be killed.
> >
> > After this change it will be killed or not. It won't be killed if
> > do_select() finds an already ready fd without blocking, or it finds a
> > ready fd right after SIGINT interrupts poll_schedule_timeout().
> 
> Yes.  Because having the signal set in real_blocked disables the
> immediate kill optimization, and the signal has to be delivered before
> we decide to kill the process.  Which matters because as you say if
> nothing checks signal_pending() when the signals are unblocked we might
> not attempt to deliver the signal.
> 
> So it is a matter of timing.
> 
> If we have both a signal and a file descriptor become ready
> at the same time I would call that a race.  Either could
> wake up the process and depending on the exact time we could
> return either one.
> 
> So it is possible that today if the signal came just after the file
> descriptor ,the code might have made it to restore_saved_sigmask_unless,
> before __send_signal runs.
> 
> I see the concern.  I think in a matter like this we try it.  Make
> the patches clean so people can bisect the problem.  Then if someone
> runs into this problem we revert the offending patches.

If I have an application that has a loop with a pselect call that
enables SIGINT (without a handler) and, for whatever reason,
one of the fd is always 'ready' then I'd expect a SIGINT
(from ^C) to terminate the program.

A quick test program:

#include <sys/time.h>
#include <sys/types.h>
#include <unistd.h>

#include <sys/select.h>
#include <signal.h>

int main(int argc, char **argv)
{
        fd_set readfds;
        sigset_t sig_int;
        struct timespec delay = {1, 0};

        sigfillset(&sig_int);
        sigdelset(&sig_int, SIGINT);

        sighold(SIGINT);

        for (;;) {
                FD_ZERO(&readfds);
                FD_SET(0, &readfds);
                pselect(1, &readfds, NULL, NULL, &delay, &sig_int);

                poll(0,0,1000);
        }
}

Run under strace to see what is happening and send SIGINT from a different terminal.
The program sleeps for a second in each of the pselect() and poll() calls.
Send a SIGINT and in terminates after pselect() returns ERESTARTNOHAND.

Run again, this time press enter - making fd 0 readable.
pselect() returns 1, but the program still exits.
(Tested on a 5.1.0-rc5 kernel.)

If a signal handler were defined it should be called instead.

FWIW is ERESTARTNOHAND actually sane here?
If I've used setitimer() to get SIGALARM generated every second I'd
expect select() to return EINTR every second even if I don't
have a SIGALARM handler?

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)