ACK: [SRU][Bionic][Cosmic][PATCH 1/1] powerpc/vdso: Correct call frame information

[Kleber Souza]

On 10/16/18 16:17, Joseph Salisbury wrote:
> From: Alan Modra <amodra at gmail.com>
> 
> BugLink: https://bugs.launchpad.net/bugs/1797963
> 
> Call Frame Information is used by gdb for back-traces and inserting
> breakpoints on function return for the "finish" command.  This failed
> when inside __kernel_clock_gettime.  More concerning than difficulty
> debugging is that CFI is also used by stack frame unwinding code to
> implement exceptions.  If you have an app that needs to handle
> asynchronous exceptions for some reason, and you are unlucky enough to
> get one inside the VDSO time functions, your app will crash.
> 
> What's wrong:  There is control flow in __kernel_clock_gettime that
> reaches label 99 without saving lr in r12.  CFI info however is
> interpreted by the unwinder without reference to control flow: It's a
> simple matter of "Execute all the CFI opcodes up to the current
> address".  That means the unwinder thinks r12 contains the return
> address at label 99.  Disabuse it of that notion by resetting CFI for
> the return address at label 99.
> 
> Note that the ".cfi_restore lr" could have gone anywhere from the
> "mtlr r12" a few instructions earlier to the instruction at label 99.
> I put the CFI as late as possible, because in general that's best
> practice (and if possible grouped with other CFI in order to reduce
> the number of CFI opcodes executed when unwinding).  Using r12 as the
> return address is perfectly fine after the "mtlr r12" since r12 on
> that code path still contains the return address.
> 
> __get_datapage also has a CFI error.  That function temporarily saves
> lr in r0, and reflects that fact with ".cfi_register lr,r0".  A later
> use of r0 means the CFI at that point isn't correct, as r0 no longer
> contains the return address.  Fix that too.
> 
> Signed-off-by: Alan Modra <amodra at gmail.com>
> Tested-by: Reza Arbab <arbab at linux.ibm.com>
> Signed-off-by: Paul Mackerras <paulus at ozlabs.org>
> (cherry picked from commit 56d20861c027498b5a1112b4f9f05b56d906fdda linux-next)
> Signed-off-by: Joseph Salisbury <joseph.salisbury at canonical.com>

Acked-by: Kleber Sacilotto de Souza <kleber.souza at canonical.com>

> 
> ---
>  arch/powerpc/kernel/vdso32/datapage.S     | 1 +
>  arch/powerpc/kernel/vdso32/gettimeofday.S | 1 +
>  arch/powerpc/kernel/vdso64/datapage.S     | 1 +
>  arch/powerpc/kernel/vdso64/gettimeofday.S | 1 +
>  4 files changed, 4 insertions(+)
> 
> diff --git a/arch/powerpc/kernel/vdso32/datapage.S b/arch/powerpc/kernel/vdso32/datapage.S
> index 3745113..2a7eb54 100644
> --- a/arch/powerpc/kernel/vdso32/datapage.S
> +++ b/arch/powerpc/kernel/vdso32/datapage.S
> @@ -37,6 +37,7 @@ data_page_branch:
>  	mtlr	r0
>  	addi	r3, r3, __kernel_datapage_offset-data_page_branch
>  	lwz	r0,0(r3)
> +  .cfi_restore lr
>  	add	r3,r0,r3
>  	blr
>    .cfi_endproc
> diff --git a/arch/powerpc/kernel/vdso32/gettimeofday.S b/arch/powerpc/kernel/vdso32/gettimeofday.S
> index 769c262..1e0bc59 100644
> --- a/arch/powerpc/kernel/vdso32/gettimeofday.S
> +++ b/arch/powerpc/kernel/vdso32/gettimeofday.S
> @@ -139,6 +139,7 @@ V_FUNCTION_BEGIN(__kernel_clock_gettime)
>  	 */
>  99:
>  	li	r0,__NR_clock_gettime
> +  .cfi_restore lr
>  	sc
>  	blr
>    .cfi_endproc
> diff --git a/arch/powerpc/kernel/vdso64/datapage.S b/arch/powerpc/kernel/vdso64/datapage.S
> index abf17fe..bf96686 100644
> --- a/arch/powerpc/kernel/vdso64/datapage.S
> +++ b/arch/powerpc/kernel/vdso64/datapage.S
> @@ -37,6 +37,7 @@ data_page_branch:
>  	mtlr	r0
>  	addi	r3, r3, __kernel_datapage_offset-data_page_branch
>  	lwz	r0,0(r3)
> +  .cfi_restore lr
>  	add	r3,r0,r3
>  	blr
>    .cfi_endproc
> diff --git a/arch/powerpc/kernel/vdso64/gettimeofday.S b/arch/powerpc/kernel/vdso64/gettimeofday.S
> index 3820213..09b2a49 100644
> --- a/arch/powerpc/kernel/vdso64/gettimeofday.S
> +++ b/arch/powerpc/kernel/vdso64/gettimeofday.S
> @@ -124,6 +124,7 @@ V_FUNCTION_BEGIN(__kernel_clock_gettime)
>  	 */
>  99:
>  	li	r0,__NR_clock_gettime
> +  .cfi_restore lr
>  	sc
>  	blr
>    .cfi_endproc
>