[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Fri, 22 Jan 2021 09:44:05 -0500
From: Rich Felker <dalias@...c.org>
To: Florian Weimer <fweimer@...hat.com>
Cc: Nicholas Piggin <npiggin@...il.com>, linuxppc-dev@...ts.ozlabs.org,
Alan Modra <amodra@...il.com>, musl@...ts.openwall.com,
libc-alpha@...rceware.org
Subject: Re: Re: [PATCH v2] powerpc/64/signal: balance return
predictor stack in signal trampoline
On Fri, Jan 22, 2021 at 12:27:14PM +0100, Florian Weimer wrote:
> * 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?
Maybe I'm missing something but I don't see how this would break musl;
we just inspect the PC in the mcontext, which I don't see any changes
to and which should still point to the next instruction of the
interrupted context. I don't have a test environment though so I'll
have to wait for feedback from ppc users to be sure. Are there any
further details on how it's breaking glibc?
Rich
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.
