RE: [PATCH v2] signal: Adjust error codes according to restore_user_sigmask()

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

From: Deepa Dinamani
> Sent: 28 May 2019 12:38
> > On May 28, 2019, at 2:12 AM, David Laight <David.Laight@aculab.com> wrote:
> >
> > From: Deepa Dinamani
> >> Sent: 24 May 2019 18:02
> > ...
> >> Look at the code before 854a6ed56839a:
> >>
> >> /*
> >>       * If we changed the signal mask, we need to restore the original one.
> >>       * In case we've got a signal while waiting, we do not restore the
> >>       * signal mask yet, and we allow do_signal() to deliver the signal on
> >>       * the way back to userspace, before the signal mask is restored.
> >>       */
> >>      if (sigmask) {
> >>             ####### This err has not been changed since ep_poll()
> >>             ####### So if there is a signal before this point, but
> >> err = 0, then we goto else.
> >>              if (err == -EINTR) {
> >>                      memcpy(&current->saved_sigmask, &sigsaved,
> >>                             sizeof(sigsaved));
> >>                      set_restore_sigmask();
> >>              } else
> >>                    ############ This is a problem if there is signal
> >> pending that is sigmask should block.
> >>                    ########### This is the whole reason we have
> >> current->saved_sigmask?
> >>                      set_current_blocked(&sigsaved);
> >>      }
> >
> > What happens if all that crap is just deleted (I presume from the
> > bottom of ep_wait()) ?
> 
> Hmm, you have to update the saved_sigmask or the sigmask.

Doesn't the top of ep_poll() do all that?
Something like copying the current sigmask to the saved_sigmask
and the user-supplied sigmask to the current sigmask.
Otherwise the sleeps won't be interruptible.

It is worth noting that both pselect() and epoll_pwait() differ
from sigwait() (and friends) which were (IIRC) the first system
calls to try to remove the timing windows associated with waiting
for signals.
sigwait() returns the signal number and removes it from the pending
mask - so the signal handler won't be called for the returned signal.
(Unless it wasn't actually masked!)
So the sigwait() code does have to restore the signal mask itself.

> > I'm guessing that on the way back to userspace signal handlers for
> > signals enabled in the process's current mask (the one specified
> > to epoll_pwait) get called.
> > Then the signal mask is loaded from current->saved_sigmask and
> > and enabled signal handlers are called again.
> 
> Who is saving this saved_sigmask that is being restored on the way back?

It has to be saved when the user-supplied one in installed.

> > No special code there that depends on the syscall result, errno
> > of the syscall number.
> 
> I didn’t say this has anything to do with errno.

The code snippet above checks it ...

	David

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