linux-kernel - Re: [PATCH 1/8] ARM: ARMv7-M: Fix register restore corrupt after svc call

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 3 Mar 2021 09:52:07 +0000
From:   Vladimir Murzin <vladimir.murzin@....com>
To:     dillon.minfei@...il.com, robh+dt@...nel.org,
        mcoquelin.stm32@...il.com, alexandre.torgue@...com,
        devicetree@...r.kernel.org,
        linux-stm32@...md-mailman.stormreply.com,
        linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
        linux@...linux.org.uk, afzal.mohd.ma@...il.com
Subject: Re: [PATCH 1/8] ARM: ARMv7-M: Fix register restore corrupt after svc
 call

On 3/3/21 8:05 AM, dillon.minfei@...il.com wrote:
> From: dillon min <dillon.minfei@...il.com>
> 
> For some case, kernel not boot by u-boot(single thread),
> but by rtos , as most rtos use pendsv to do context switch.


Hmm, does it mean that it starts kernel from process context?

I'd assume that it is not only kernel who expects MSP. So, what
if RTOS you mentioned want to boot other RTOS (even itself)? What
if you have no access to the source code for those RTOS(es) to
patch MSP/PSP switch?

I'd very much prefer to keep stack switching logic outside kernel,
say, in some shim which RTOS/bootloader can maintain.

Cheers
Vladimir

> 
> So, we need add an lr check after svc call, to find out should
> use psp or msp. else register restore after svc call might be
> corrupted.
> 
> Fixes: b70cd406d7fe ("ARM: 8671/1: V7M: Preserve registers across switch from Thread to Handler mode")
> Signed-off-by: dillon min <dillon.minfei@...il.com>
> ---
>  arch/arm/mm/proc-v7m.S | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/mm/proc-v7m.S b/arch/arm/mm/proc-v7m.S
> index 84459c1d31b8..c93d2757312d 100644
> --- a/arch/arm/mm/proc-v7m.S
> +++ b/arch/arm/mm/proc-v7m.S
> @@ -137,7 +137,10 @@ __v7m_setup_cont:
>  1:	cpsid	i
>  	/* Calculate exc_ret */
>  	orr	r10, lr, #EXC_RET_THREADMODE_PROCESSSTACK
> -	ldmia	sp, {r0-r3, r12}
> +	tst	lr, #EXC_RET_STACK_MASK
> +	mrsne	r4, psp
> +	moveq	r4, sp
> +	ldmia	r4!, {r0-r3, r12}
>  	str	r5, [r12, #11 * 4]	@ restore the original SVC vector entry
>  	mov	lr, r6			@ restore LR
>  
>