[PATCH] nptl: Use SA_RESTART for SIGCANCEL handler
Florian Weimer
fweimer@redhat.com
Fri Jun 18 11:38:26 GMT 2021
* Adhemerval Zanella via Libc-alpha:
> The usage of signals to implementation pthread cancellation is an
> implementation detail and should not be visible through cancellation
> entrypoints.
>
> However now that pthread_cancel always send the SIGCANCEL, some
> entrypoint might be interruptable and return EINTR to the caller
> (for instance on sem_wait).
>
> Using SA_RESTART hides this, since the cancellation handler should
> either act uppon cancellation (if asynchronous cancellation is enable)
> or ignore the cancellation internal signal.
I think this still needs a NEWS entry because there have been kernel
bugs in this area (e.g. in CIFS).
> Checked on x86_64-linux-gnu and i686-linux-gnu.
> ---
> nptl/pthread_cancel.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/nptl/pthread_cancel.c b/nptl/pthread_cancel.c
> index 0698cd2046..cc25ff21f3 100644
> --- a/nptl/pthread_cancel.c
> +++ b/nptl/pthread_cancel.c
> @@ -72,7 +72,11 @@ __pthread_cancel (pthread_t th)
> {
> struct sigaction sa;
> sa.sa_sigaction = sigcancel_handler;
> - sa.sa_flags = SA_SIGINFO;
> + /* The signal handle should be non-interruptible to avoid the risk of
> + spurious EINTR caused by SIGCANCEL sent to process or if
> + pthread_cancel() is called while cancellation is disabled in the
> + target thread. */
> + sa.sa_flags = SA_SIGINFO | SA_RESTART;
> __sigemptyset (&sa.sa_mask);
> __libc_sigaction (SIGCANCEL, &sa, NULL);
> atomic_store_relaxed (&init_sigcancel, 1);
I really don't feel comfortable reviewing this. However I think it is
still consistent with the (buggy) SYSCALL_CANCEL implementation:
int sc_cancel_oldtype = LIBC_CANCEL_ASYNC (); \
sc_ret = INLINE_SYSCALL_CALL (__VA_ARGS__); \
LIBC_CANCEL_RESET (sc_cancel_oldtype); \
We temporary enable async cancellation, in which case we unwind through
the signal handler if canceled. We do not rely on a EINTR error return
from the system call and a cancellation check outside of the signal
handler. So adding SA_RESTART should really be okay.
Thanks,
Florian
More information about the Libc-alpha
mailing list