stable.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: David Laight <David.Laight@ACULAB.COM>
To: "'Eric W. Biederman'" <ebiederm@xmission.com>,
	Oleg Nesterov <oleg@redhat.com>
Cc: Deepa Dinamani <deepa.kernel@gmail.com>,
	Al Viro <viro@ZenIV.linux.org.uk>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"akpm@linux-foundation.org" <akpm@linux-foundation.org>,
	"arnd@arndb.de" <arnd@arndb.de>,
	"dbueso@suse.de" <dbueso@suse.de>,
	"axboe@kernel.dk" <axboe@kernel.dk>,
	"dave@stgolabs.net" <dave@stgolabs.net>,
	"e@80x24.org" <e@80x24.org>,
	"jbaron@akamai.com" <jbaron@akamai.com>,
	"linux-fsdevel@vger.kernel.org" <linux-fsdevel@vger.kernel.org>,
	"linux-aio@kvack.org" <linux-aio@kvack.org>,
	"omar.kilani@gmail.com" <omar.kilani@gmail.com>,
	"tglx@linutronix.de" <tglx@linutronix.de>,
	"stable@vger.kernel.org" <stable@vger.kernel.org>
Subject: RE: pselect/etc semantics
Date: Thu, 30 May 2019 15:18:21 +0000	[thread overview]
Message-ID: <9150b8cb8123426492a82e193f45229e@AcuMS.aculab.com> (raw)
In-Reply-To: <87woi8rt96.fsf@xmission.com>

From: Eric W. Biederman
> Sent: 30 May 2019 14:01
> Oleg Nesterov <oleg@redhat.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)


  reply	other threads:[~2019-05-30 15:18 UTC|newest]

Thread overview: 95+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-05-22  3:21 [PATCH v2] signal: Adjust error codes according to restore_user_sigmask() Deepa Dinamani
2019-05-22 15:05 ` Oleg Nesterov
2019-05-22 15:55   ` Deepa Dinamani
2019-05-22 16:14     ` Oleg Nesterov
2019-05-22 16:33       ` Deepa Dinamani
2019-05-23  9:03         ` David Laight
2019-05-23 14:59           ` Oleg Nesterov
2019-05-23 16:18             ` David Laight
2019-05-23 16:36               ` Oleg Nesterov
2019-05-23 16:56                 ` David Laight
2019-05-23 18:06                   ` Deepa Dinamani
2019-05-23 20:41                     ` Deepa Dinamani
2019-05-23 21:06                       ` Deepa Dinamani
2019-05-24  9:58                     ` David Laight
2019-05-24 14:10                     ` Oleg Nesterov
2019-05-24 15:16                       ` Deepa Dinamani
2019-05-24 16:33                         ` Oleg Nesterov
2019-05-24 17:01                           ` Deepa Dinamani
2019-05-27 15:04                             ` Oleg Nesterov
2019-05-28 20:47                               ` Deepa Dinamani
2019-05-29 16:57                                 ` Oleg Nesterov
2019-05-29 18:42                                   ` Deepa Dinamani
2019-05-28  9:02                             ` David Laight
2019-05-28  9:12                             ` David Laight
2019-05-28 11:37                               ` Deepa Dinamani
2019-05-28 12:04                                 ` David Laight
2019-05-24 14:19                     ` Oleg Nesterov
2019-05-24 14:29                       ` Deepa Dinamani
2019-05-24 14:51                         ` Oleg Nesterov
2019-05-24 13:29                   ` Oleg Nesterov
2019-05-24 14:59                     ` David Laight
2019-05-24 15:09                       ` David Laight
2019-05-24 15:46                         ` Oleg Nesterov
2019-05-24 15:44                       ` Oleg Nesterov
2019-05-24 16:40                         ` David Laight
2019-05-23 14:33         ` Oleg Nesterov
2019-05-22 22:18 ` Chris Down
2019-05-22 22:52   ` Deepa Dinamani
2019-05-29 16:11 ` pselect/etc semantics (Was: [PATCH v2] signal: Adjust error codes according to restore_user_sigmask()) Oleg Nesterov
2019-05-29 16:54   ` David Laight
2019-05-29 18:50     ` Eric Wong
2019-05-30  9:34       ` David Laight
2019-05-30 13:04       ` pselect/etc semantics Eric W. Biederman
2019-05-29 16:56   ` pselect/etc semantics (Was: [PATCH v2] signal: Adjust error codes according to restore_user_sigmask()) Deepa Dinamani
2019-05-29 18:26   ` Deepa Dinamani
2019-05-29 22:32   ` Arnd Bergmann
2019-05-30  1:54     ` pselect/etc semantics Eric W. Biederman
2019-05-30 18:28       ` Arnd Bergmann
2019-05-30 14:40     ` pselect/etc semantics (Was: [PATCH v2] signal: Adjust error codes according to restore_user_sigmask()) Oleg Nesterov
2019-05-30 18:37       ` Arnd Bergmann
2019-05-30 13:01   ` pselect/etc semantics Eric W. Biederman
2019-05-30 15:18     ` David Laight [this message]
2019-05-30 16:13       ` Oleg Nesterov
2019-05-30 15:38     ` Eric W. Biederman
2019-05-30 15:48       ` Deepa Dinamani
2019-05-30 16:59         ` Deepa Dinamani
2019-05-30 16:08       ` Oleg Nesterov
2019-05-30 17:20         ` Eric W. Biederman
2019-05-30 16:22       ` David Laight
2019-05-30 15:57     ` Oleg Nesterov
2019-05-30 21:03     ` Eric Wong
2019-06-04 13:41   ` [PATCH] signal: remove the wrong signal_pending() check in restore_user_sigmask() Oleg Nesterov
2019-06-04 15:31     ` Eric W. Biederman
2019-06-04 15:57       ` David Laight
2019-06-04 16:37     ` Arnd Bergmann
2019-06-04 18:14       ` Deepa Dinamani
2019-06-04 18:35     ` Eric Wong
2019-06-04 21:26     ` Linus Torvalds
2019-06-04 22:24       ` Eric Wong
2019-06-04 23:51       ` Eric W. Biederman
2019-06-05  9:04         ` Oleg Nesterov
2019-06-05  8:56       ` Oleg Nesterov
2019-06-05  9:02       ` David Laight
2019-06-05  9:25         ` Oleg Nesterov
2019-06-05  9:58           ` David Laight
2019-06-05 15:58     ` [PATCH -mm 0/1] signal: simplify set_user_sigmask/restore_user_sigmask Oleg Nesterov
2019-06-05 15:58       ` [PATCH -mm 1/1] " Oleg Nesterov
2019-06-06  0:14         ` kbuild test robot
2019-06-06  1:06         ` kbuild test robot
2019-06-06  7:25         ` Oleg Nesterov
2019-06-06  7:30           ` Sedat Dilek
2019-06-05 17:24       ` [PATCH -mm 0/1] " Linus Torvalds
2019-06-06  9:05         ` David Laight
2019-06-06 11:05           ` Oleg Nesterov
2019-06-06 11:29             ` David Laight
2019-06-06 12:41               ` Oleg Nesterov
2019-06-06 13:23                 ` David Laight
2019-06-06 10:22         ` Oleg Nesterov
2019-06-06 11:32       ` [PATCH -mm V2 1/1] " Oleg Nesterov
2019-06-06 14:08     ` [PATCH 0/2] select: simplify the usage of restore_saved_sigmask_unless() Oleg Nesterov
2019-06-06 14:08       ` [PATCH 1/2] select: change do_poll() to return -ERESTARTNOHAND rather than -EINTR Oleg Nesterov
2019-06-07 18:05         ` Linus Torvalds
2019-06-06 14:09       ` [PATCH 2/2] select: shift restore_saved_sigmask_unless() into poll_select_copy_remaining() Oleg Nesterov
2019-06-07 21:39       ` [RFC PATCH 0/5]: Removing saved_sigmask Eric W. Biederman
2019-06-11 18:58         ` Oleg Nesterov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=9150b8cb8123426492a82e193f45229e@AcuMS.aculab.com \
    --to=david.laight@aculab.com \
    --cc=akpm@linux-foundation.org \
    --cc=arnd@arndb.de \
    --cc=axboe@kernel.dk \
    --cc=dave@stgolabs.net \
    --cc=dbueso@suse.de \
    --cc=deepa.kernel@gmail.com \
    --cc=e@80x24.org \
    --cc=ebiederm@xmission.com \
    --cc=jbaron@akamai.com \
    --cc=linux-aio@kvack.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=oleg@redhat.com \
    --cc=omar.kilani@gmail.com \
    --cc=stable@vger.kernel.org \
    --cc=tglx@linutronix.de \
    --cc=torvalds@linux-foundation.org \
    --cc=viro@ZenIV.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).