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.