linux-kernel - Re: [PATCH v5 1/2] arm64: adjust el0_sync so that a function can be called

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, 28 May 2014 12:35:51 -0700
From:	Larry Bassel <larry.bassel@...aro.org>
To:	Will Deacon <will.deacon@....com>
Cc:	Larry Bassel <larry.bassel@...aro.org>,
	Catalin Marinas <Catalin.Marinas@....com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>,
	"linaro-kernel@...ts.linaro.org" <linaro-kernel@...ts.linaro.org>,
	"khilman@...aro.org" <khilman@...aro.org>
Subject: Re: [PATCH v5 1/2] arm64: adjust el0_sync so that a function can
 be called

On 28 May 14 12:27, Will Deacon wrote:
> Hi Larry,
> 
> On Mon, May 26, 2014 at 07:56:12PM +0100, Larry Bassel wrote:
> > To implement the context tracker properly on arm64,
> > a function call needs to be made after debugging and
> > interrupts are turned on, but before the lr is changed
> > to point to ret_to_user(). If the function call
> > is made after the lr is changed the function will not
> > return to the correct place.
> > 
> > For similar reasons, defer the setting of x0 so that
> > it doesn't need to be saved around the function call
> > (save far_el1 in x26 temporarily instead).
> > 
> > Signed-off-by: Larry Bassel <larry.bassel@...aro.org>
> 
> [...]
> 
> 
> Why have you added this mov instruction?

I believe (please correct me if I'm wrong) that it is necessary.
Here is why:

> > @@ -476,23 +481,27 @@ el0_undef:
> >  	// enable interrupts before calling the main handler
> >  	enable_dbg_and_irq
> >  	mov	x0, sp
> > +	adr	lr, ret_to_user
> >  	b	do_undefinstr
> >  el0_dbg:
> >  	/*
> >  	 * Debug exception handling
> >  	 */
> >  	tbnz	x24, #0, el0_inv		// EL0 only
> > -	mrs	x0, far_el1
> > +	mrs	x26, far_el1

needed because do_debug_exception may clobber x0, so save far_el1
in x26 (as other parts of this patch do)

> > +	mov	x0, x26

needed because far_el1 is expected to be in x0 here

> >  	mov	x1, x25
> >  	mov	x2, sp
> >  	bl	do_debug_exception
> >  	enable_dbg

[call to ct_user_exit will go here in the next patch, this may re-clobber x0]

> > +	mov	x0, x26

needed because far_el1 is expected to be in x0 here

Since the purpose of this patch is to make calling a function
possible in this code path, the "extra" mov instruction above
is necessary and IMHO should be added in this patch and not in the
next one whose purpose is to define the ct_user_* macros and
add calls to them in the proper places.

> >  	b	ret_to_user
> 
> Will

Larry
--
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/