Re: [PATCH] signal: remove the wrong signal_pending() check in restore_user_sigmask()

From: Oleg Nesterov
Date: Wed Jun 05 2019 - 05:29:32 EST


On 06/05, David Laight wrote:
>
> epoll() would have:
> if (restore_user_sigmask(xxx.sigmask, &sigsaved, !ret || ret == -EINTR))
> ret = -EINTR;

I don't think so but lets discuss this later.

> I also think it could be simplified if code that loaded the 'user sigmask'
> saved the old one in 'current->saved_sigmask' (and saved that it had done it).
> You'd not need 'sigsaved' nor pass the user sigmask address into
> the restore function.

Heh. apparently you do not read my emails ;)

This is what I proposed in my very 1st email, and I even showed the patch
and the code with the patch applied twice. Let me do this again.

Let me show the code with the patch applied. I am using epoll_pwait() as an
example because it looks very simple.


static inline void set_restore_sigmask(void)
{
// WARN_ON(!TIF_SIGPENDING) was removed by this patch
current->restore_sigmask = true;
}

int set_xxx(const sigset_t __user *umask, size_t sigsetsize)
{
sigset_t *kmask;

if (!umask)
return 0;
if (sigsetsize != sizeof(sigset_t))
return -EINVAL;
if (copy_from_user(kmask, umask, sizeof(sigset_t)))
return -EFAULT;

// we can safely modify ->saved_sigmask/restore_sigmask, they has no meaning
// until the syscall returns.
set_restore_sigmask();
current->saved_sigmask = current->blocked;
set_current_blocked(kmask);

return 0;
}


void update_xxx(bool interrupted)
{
// the main reason for this helper is WARN_ON(!TIF_SIGPENDING) which was "moved"
// from set_restore_sigmask() above.
if (interrupted)
WARN_ON(!test_thread_flag(TIF_SIGPENDING));
else
restore_saved_sigmask();
}

SYSCALL_DEFINE6(epoll_pwait, int, epfd, struct epoll_event __user *, events,
int, maxevents, int, timeout, const sigset_t __user *, sigmask,
size_t, sigsetsize)
{
int error;

error = set_xxx(sigmask, sigsetsize);
if (error)
return error;

error = do_epoll_wait(epfd, events, maxevents, timeout);
update_xxx(error == -EINTR);

return error;
}

Oleg.