On 05/30, Eric W. Biederman wrote:
>
> Oleg Nesterov <oleg@xxxxxxxxxx> 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.
