linux-kernel - RE: [PATCH v2] signal: Adjust error codes according to restore_user

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 28 May 2019 12:04:17 +0000
From:   David Laight <David.Laight@...LAB.COM>
To:     'Deepa Dinamani' <deepa.kernel@...il.com>
CC:     Oleg Nesterov <oleg@...hat.com>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Alexander Viro <viro@...iv.linux.org.uk>,
        Arnd Bergmann <arnd@...db.de>,
        "dbueso@...e.de" <dbueso@...e.de>,
        "axboe@...nel.dk" <axboe@...nel.dk>,
        Davidlohr Bueso <dave@...olabs.net>, Eric Wong <e@...24.org>,
        Jason Baron <jbaron@...mai.com>,
        Linux FS-devel Mailing List <linux-fsdevel@...r.kernel.org>,
        linux-aio <linux-aio@...ck.org>,
        Omar Kilani <omar.kilani@...il.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        "stable@...r.kernel.org" <stable@...r.kernel.org>
Subject: RE: [PATCH v2] signal: Adjust error codes according to
 restore_user_sigmask()

From: Deepa Dinamani
> Sent: 28 May 2019 12:38
> > On May 28, 2019, at 2:12 AM, David Laight <David.Laight@...lab.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)