Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-ID: <87im7pp5yl.fsf@oldenburg.str.redhat.com>
Date: Fri, 22 Jan 2021 12:27:14 +0100
From: Florian Weimer <fweimer@...hat.com>
To: Nicholas Piggin <npiggin@...il.com>
Cc: linuxppc-dev@...ts.ozlabs.org,  Alan Modra <amodra@...il.com>,
 musl@...ts.openwall.com, libc-alpha@...rceware.org
Subject: Re: [PATCH v2] powerpc/64/signal: balance return predictor stack in
 signal trampoline

* Nicholas Piggin:

> diff --git a/arch/powerpc/kernel/vdso64/sigtramp.S b/arch/powerpc/kernel/vdso64/sigtramp.S
> index a8cc0409d7d2..bbf68cd01088 100644
> --- a/arch/powerpc/kernel/vdso64/sigtramp.S
> +++ b/arch/powerpc/kernel/vdso64/sigtramp.S
> @@ -6,6 +6,7 @@
>   * Copyright (C) 2004 Benjamin Herrenschmuidt (benh@...nel.crashing.org), IBM Corp.
>   * Copyright (C) 2004 Alan Modra (amodra@...ibm.com)), IBM Corp.
>   */
> +#include <asm/cache.h>		/* IFETCH_ALIGN_BYTES */
>  #include <asm/processor.h>
>  #include <asm/ppc_asm.h>
>  #include <asm/unistd.h>
> @@ -14,21 +15,17 @@
>  
>  	.text
>  
> -/* The nop here is a hack.  The dwarf2 unwind routines subtract 1 from
> -   the return address to get an address in the middle of the presumed
> -   call instruction.  Since we don't have a call here, we artificially
> -   extend the range covered by the unwind info by padding before the
> -   real start.  */
> -	nop
>  	.balign 8
> +	.balign IFETCH_ALIGN_BYTES
>  V_FUNCTION_BEGIN(__kernel_sigtramp_rt64)
> -.Lsigrt_start = . - 4
> +.Lsigrt_start:
> +	bctrl	/* call the handler */
>  	addi	r1, r1, __SIGNAL_FRAMESIZE
>  	li	r0,__NR_rt_sigreturn
>  	sc
>  .Lsigrt_end:
>  V_FUNCTION_END(__kernel_sigtramp_rt64)
> -/* The ".balign 8" above and the following zeros mimic the old stack
> +/* The .balign 8 above and the following zeros mimic the old stack
>     trampoline layout.  The last magic value is the ucontext pointer,
>     chosen in such a way that older libgcc unwind code returns a zero
>     for a sigcontext pointer.  */

As far as I understand it, this breaks cancellation handling on musl and
future glibc because it is necessary to look at the signal delivery
location to see if a system call sequence has result in an action, and
that location is no longer in user code after this change.

We have a glibc test in preparation of our change, and it started
failing:

  Linux 5.10 breaks sigcontext_get_pc on powerpc64
  <https://sourceware.org/bugzilla/show_bug.cgi?id=27223>

Isn't it possible to avoid the return predictor desynchronization by
adding the appropriate hint?

Thanks,
Florian
-- 
Red Hat GmbH, https://de.redhat.com/ , Registered seat: Grasbrunn,
Commercial register: Amtsgericht Muenchen, HRB 153243,
Managing Directors: Charles Cachera, Brian Klemm, Laurie Krebs, Michael O'Neill

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.