Re: pselect/etc semantics

[Oleg Nesterov <oleg@redhat.com>]

On 05/30, Eric W. Biederman wrote:
>
> 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.

I didn't even try to investigate, I wasn't cc'ed initially and I then I had
enough confusion when I started to look at the patch.

However, 854a6ed56839a40f6 obviously introduced the user-visible change so
I am not surprised it breaks something. And yes, personally I think this
change is not right.

> Which means I believe we have a semantically valid change in behavior
> that is causing a regression.

See below,

> void restore_user_sigmask(const void __user *usigmask, sigset_t *sigsaved)
> {
>
> 	if (!usigmask)
> 		return;
> 	/*
> 	 * When signals are pending, do not restore them here.
> 	 * Restoring sigmask here can lead to delivering signals that the above
> 	 * syscalls are intended to block because of the sigmask passed in.
> 	 */
> 	if (signal_pending(current)) {
> 		current->saved_sigmask = *sigsaved;
> 		set_restore_sigmask();
> 		return;
> 	}
>
> 	/*
> 	 * This is needed because the fast syscall return path does not restore
> 	 * saved_sigmask when signals are not pending.
> 	 */
> 	set_current_blocked(sigsaved);
> }
>
> Which has been reported results in a return value of 0,

0 or success

> and a signal
> delivered, where previously that did not happen.

Yes.

And to me this doesn't look right. OK, OK, probably this is because I never
read the docs, only the source code in fs/select.c. But to me pselect() should
either return success/timeout or deliver a signal. Not both. Even if the signal
was already pending before pselect() was called.

To clarify, "a signal" means a signal which was blocked before pselect(sigmask)
and temporary unblocked in this syscall.

And even if this doesn't violate posix, I see no reason to change the historic
behaviour. And this regression probably means we can't ;)

Oleg.