diff mbox series

nptl: Install SIGSETXID handler with SA_ONSTACK [BZ #27914]

Message ID 87fsyat4gs.fsf@oldenburg.str.redhat.com
State Committed
Commit 76b0c59e333da6fd4103e478b63522636d9d232b
Headers
Series nptl: Install SIGSETXID handler with SA_ONSTACK [BZ #27914] |

Checks

Context Check Description
dj/TryBot-apply_patch success Patch applied to master at the time it was sent

Commit Message

Florian Weimer May 26, 2021, 6:05 a.m. UTC 
  The signal is sent to all threads, some of which may have switched
to very small stacks.  If they have also installed an alternate
signal stack, SA_ONSTACK makes this work.  The Go runtime needs this:

  runtime: C.setuid/C.setgid smashes Go stack
  <https://github.com/golang/go/issues/9400>

Doing this for SIGCANCEL is less obviously beneficial and needs further
testing.

Tested on i686-linux-gnu and x86_64-linux-gnu.  I verified that a
glibc build for a distribution unbreaks the Go test suite on x86-64.

---
 nptl/pthread_create.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

Comments

Carlos O'Donell May 27, 2021, 12:50 p.m. UTC | #1 
On 5/26/21 2:05 AM, Florian Weimer via Libc-alpha wrote:
> The signal is sent to all threads, some of which may have switched
> to very small stacks.  If they have also installed an alternate
> signal stack, SA_ONSTACK makes this work.  The Go runtime needs this:
> 
>   runtime: C.setuid/C.setgid smashes Go stack
>   <https://github.com/golang/go/issues/9400>
> 
> Doing this for SIGCANCEL is less obviously beneficial and needs further
> testing.
> 
> Tested on i686-linux-gnu and x86_64-linux-gnu.  I verified that a
> glibc build for a distribution unbreaks the Go test suite on x86-64.

This looks good to me. Fundamentally all internal signals should have used
SA_ONSTACK, but we can fix that later. Thanks for testing this.

Reviewed-by: Carlos O'Donell <carlos@redhat.com>

> ---
>  nptl/pthread_create.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/nptl/pthread_create.c b/nptl/pthread_create.c
> index 5680687efe..b7073a8285 100644
> --- a/nptl/pthread_create.c
> +++ b/nptl/pthread_create.c
> @@ -83,9 +83,12 @@ late_init (void)
>        (void) __libc_sigaction (SIGCANCEL, &sa, NULL);
>      }
>  
> -  /* Install the handle to change the threads' uid/gid.  */
> +  /* Install the handle to change the threads' uid/gid.  Use
> +     SA_ONSTACK because the signal may be sent to threads that are
> +     running with custom stacks.  (This is less likely for
> +     SIGCANCEL.)  */
>    sa.sa_sigaction = __nptl_setxid_sighandler;
> -  sa.sa_flags = SA_SIGINFO | SA_RESTART;
> +  sa.sa_flags = SA_ONSTACK | SA_SIGINFO | SA_RESTART;
>    (void) __libc_sigaction (SIGSETXID, &sa, NULL);
>  
>    /* The parent process might have left the signals blocked.  Just in
>
diff mbox series

Patch

diff --git a/nptl/pthread_create.c b/nptl/pthread_create.c
index 5680687efe..b7073a8285 100644
--- a/nptl/pthread_create.c
+++ b/nptl/pthread_create.c
@@ -83,9 +83,12 @@  late_init (void)
       (void) __libc_sigaction (SIGCANCEL, &sa, NULL);
     }
 
-  /* Install the handle to change the threads' uid/gid.  */
+  /* Install the handle to change the threads' uid/gid.  Use
+     SA_ONSTACK because the signal may be sent to threads that are
+     running with custom stacks.  (This is less likely for
+     SIGCANCEL.)  */
   sa.sa_sigaction = __nptl_setxid_sighandler;
-  sa.sa_flags = SA_SIGINFO | SA_RESTART;
+  sa.sa_flags = SA_ONSTACK | SA_SIGINFO | SA_RESTART;
   (void) __libc_sigaction (SIGSETXID, &sa, NULL);
 
   /* The parent process might have left the signals blocked.  Just in