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

From: Linus Torvalds
Date: Tue Jun 04 2019 - 17:31:01 EST


On Tue, Jun 4, 2019 at 6:41 AM Oleg Nesterov <oleg@xxxxxxxxxx> wrote:
>
> This is the minimal fix for stable, I'll send cleanups later.

Ugh. I htink this is correct, but I wish we had a better and more
intuitive interface.

In particular, since restore_user_sigmask() basically wants to check
for "signal_pending()" anyway (to decide if the mask should be
restored by signal handling or by that function), I really get the
feeling that a lot of these patterns like

> - restore_user_sigmask(ksig.sigmask, &sigsaved);
> - if (signal_pending(current) && !ret)
> +
> + interrupted = signal_pending(current);
> + restore_user_sigmask(ksig.sigmask, &sigsaved, interrupted);
> + if (interrupted && !ret)
> ret = -ERESTARTNOHAND;

are wrong to begin with, and we really should aim for an interface
which says "tell me whether you completed the system call, and I'll
give you an error return if not".

How about we make restore_user_sigmask() take two return codes: the
'ret' we already have, and the return we would get if there is a
signal pending and w're currently returning zero.

IOW, I think the above could become

ret = restore_user_sigmask(ksig.sigmask, &sigsaved, ret, -ERESTARTHAND);

instead if we just made the right interface decision.

Hmm?

Linus