linux-kernel - Re: [PATCH] ARM: Fix preemption disable in iwmmxt_task

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:	Fri, 11 Jul 2014 18:09:36 +0100
From:	Catalin Marinas <catalin.marinas@....com>
To:	Sebastian Hesselbarth <sebastian.hesselbarth@...il.com>
Cc:	Russell King <linux@....linux.org.uk>,
	Jean-Francois Moine <moinejf@...e.fr>,
	Jason Cooper <jason@...edaemon.net>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] ARM: Fix preemption disable in iwmmxt_task_enable()

On Fri, Jul 11, 2014 at 10:10:13AM +0100, Sebastian Hesselbarth wrote:
> commit 431a84b1a4f7d1a0085d5b91330c5053cc8e8b12
>  ("ARM: 8034/1: Disable preemption in iwmmxt_task_enable()")
> introduced macros {inc,dec}_preempt_count to iwmmxt_task_enable
> to make it run with preemption disabled.
> 
> Unfortunately, other functions in iwmmxt.S also use concan_{save,dump,load}
> sections located in iwmmxt_task_enable() to deal with iWMMXt coprocessor.
> This causes an unbalanced preempt_count due to excessive dec_preempt_count
> and destroyed return addresses in callers of concan_ labels due to a register
> collision:

Indeed, I missed this part completely.

> --- a/arch/arm/kernel/iwmmxt.S
> +++ b/arch/arm/kernel/iwmmxt.S
> @@ -94,13 +94,19 @@ ENTRY(iwmmxt_task_enable)
>  
>  	mrc	p15, 0, r2, c2, c0, 0
>  	mov	r2, r2				@ cpwait
> +	bl	concan_save
>  
> -	teq	r1, #0				@ test for last ownership
> -	mov	lr, r9				@ normal exit from exception
> -	beq	concan_load			@ no owner, skip save
> +#ifdef CONFIG_PREEMPT_COUNT
> +	get_thread_info r10
> +#endif
> +4:	dec_preempt_count r10, r3
> +	mov	pc, r9				@ normal exit from exception
>  
>  concan_save:
>  
> +	teq	r1, #0				@ test for last ownership
> +	beq	concan_load			@ no owner, skip save
> +
>  	tmrc	r2, wCon
>  
>  	@ CUP? wCx
> @@ -175,10 +181,6 @@ concan_load:
>  	tmcr	wCon, r2
>  
>  3:
> -#ifdef CONFIG_PREEMPT_COUNT
> -	get_thread_info r10
> -#endif
> -4:	dec_preempt_count r10, r3
>  	mov	pc, lr

It looks fine to me. One optimisation you could do is to replace a
couple of beq 3f with moveq pc, lr.

-- 
Catalin
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/