linux-kernel - RE: pselect/etc semantics

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]
Message-ID: <9150b8cb8123426492a82e193f45229e@AcuMS.aculab.com>
Date:   Thu, 30 May 2019 15:18:21 +0000
From:   David Laight <David.Laight@...LAB.COM>
To:     "'Eric W. Biederman'" <ebiederm@...ssion.com>,
        Oleg Nesterov <oleg@...hat.com>
CC:     Deepa Dinamani <deepa.kernel@...il.com>,
        Al Viro <viro@...IV.linux.org.uk>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "akpm@...ux-foundation.org" <akpm@...ux-foundation.org>,
        "arnd@...db.de" <arnd@...db.de>, "dbueso@...e.de" <dbueso@...e.de>,
        "axboe@...nel.dk" <axboe@...nel.dk>,
        "dave@...olabs.net" <dave@...olabs.net>,
        "e@...24.org" <e@...24.org>,
        "jbaron@...mai.com" <jbaron@...mai.com>,
        "linux-fsdevel@...r.kernel.org" <linux-fsdevel@...r.kernel.org>,
        "linux-aio@...ck.org" <linux-aio@...ck.org>,
        "omar.kilani@...il.com" <omar.kilani@...il.com>,
        "tglx@...utronix.de" <tglx@...utronix.de>,
        "stable@...r.kernel.org" <stable@...r.kernel.org>
Subject: RE: pselect/etc semantics

From: Eric W. Biederman
> Sent: 30 May 2019 14:01
> Oleg Nesterov <oleg@...hat.com> writes:
> 
> > Al, Linus, Eric, please help.
> >
> > The previous discussion was very confusing, we simply can not understand each
> > other.
> >
> > To me everything looks very simple and clear, but perhaps I missed something
> > obvious? Please correct me.
> 
> If I have read this thread correctly the core issue is that ther is a
> program that used to work and that fails now.  The question is why.
> 
> There are two ways the semantics for a sigmask changing system call
> can be defined.  The naive way and by reading posix.  I will pick
> on pselect.
> 
> The naive way:
> int pselect(int nfds, fd_set *readfds, fd_set *writefds,
>             fd_set *exceptfds, const struct timespec *timeout,
>             const sigset_t *sigmask)
> {
>     sigset_t oldmask;
> 	int ret;
> 
>     if (sigmask)
> 		sigprocmask(SIG_SETMASK, sigmask, &oldmask);
> 
> 	ret = select(nfds, readfds, writefds, exceptfds, timeout);
> 
> 	if (sigmask)
> 		sigprocmask(SIG_SETMASK, &oldmask, NULL);
> 
>     return ret;
> }
> 
> The standard for pselect behavior at
> https://pubs.opengroup.org/onlinepubs/009695399/functions/pselect.html
> says:
> > If sigmask is not a null pointer, then the pselect() function shall
> > replace the signal mask of the caller by the set of signals pointed to
> > by sigmask before examining the descriptors, and shall restore the
> > signal mask of the calling thread before returning.

Right, but that bit says nothing about when signals are 'delivered'.
Section 2.4 of http://pubs.opengroup.org/onlinepubs/9699919799/functions/V2_chap02.html 
isn't very enlightening either - well, pretty impenetrable really. 
But since the ToG specs don't require a user-kernel boundary I believe
that the signal handler is expected to be called as soon as it is unmasked.

Now, for async signals, the kernel can always pretend that they happened
slightly later than they actually did.
So signal delivery is delayed until 'return to user'.
In some cases system calls are restarted to make the whole thing transparent.

For pselect() etc I think this means:

1) Signal handlers can only be called if EINTR is returned.
2) If a signal is deliverable after the mask is changed
   then the signal hander must be called.
   This means EINTR must be returned.
   ie this must be detected before checking anything else.
3) A signal that is raised after the wait completes can be
   'deemed' to have happened after the mask is restored
   and left masked until the applications next pselect() call.
4) As an optimisation a signal that arrives after the timer
   expires, but before the mask is restored can be 'deemed'
   to have happened before the timeout expired and EINTR
   returned.

Now not all system calls that use a temporary mask may want to
work that way.
In particular they may want to return 'success' and have the
signal handlers called.

So maybe the set_xxx() function that installs to user's mask
should return:
   -ve:   errno value
     0:   no signal pending
     1:   signal pending.

And the update_xxx() function that restores the saved mask
should take a parameter to indicate whether signal handlers
should be called maybe 'bool call_handlers' and return 0/1
depending on whether signals were pending.

pselect() then contains:
	rval = set_xxx();
	if (rval) {
		if (rval < 0)
			return rval;
		update_xxx(true);
		return -EINTR:
	}

	rval = wait....();

#if 0  // don't report late signals
	update_xxx(rval == -EINTR);
#else  // report signals after timeout
	if (update_xxx(!rval || rval == -EINTR))
		rval == -EINTR;
#endif

	return rval;
}

    David

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