linux-kernel - Re: [PATCH 1/4] kselftests/arm64: add a basic Pointer Authentication test

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:   Mon, 7 Sep 2020 11:23:57 +0100
From:   Dave Martin <Dave.Martin@....com>
To:     Boyan Karatotev <boyan.karatotev@....com>
Cc:     Shuah Khan <shuah@...nel.org>, boian4o1@...il.com,
        Catalin Marinas <catalin.marinas@....com>,
        linux-kernel@...r.kernel.org, linux-kselftest@...r.kernel.org,
        amit.kachhap@....com, vincenzo.frascino@....com,
        Will Deacon <will@...nel.org>,
        linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH 1/4] kselftests/arm64: add a basic Pointer Authentication
 test

On Thu, Sep 03, 2020 at 11:12:02AM +0100, Boyan Karatotev wrote:
> On 02/09/2020 17:49, Dave Martin wrote:
> > On Fri, Aug 28, 2020 at 02:16:03PM +0100, Boyan Karatotev wrote:
> >> PAuth signs and verifies return addresses on the stack. It does so by
> >> inserting a Pointer Authentication code (PAC) into some of the unused top
> >> bits of an address. This is achieved by adding paciasp/autiasp instructions
> >> at the beginning and end of a function.
> >>
> >> This feature is partially backwards compatible with earlier versions of the
> >> ARM architecture. To coerce the compiler into emitting fully backwards
> >> compatible code the main file is compiled to target an earlier ARM version.
> >> This allows the tests to check for the feature and print meaningful error
> >> messages instead of crashing.
> >>
> >> Add a test to verify that corrupting the return address results in a
> >> SIGSEGV on return.
> >>
> >> Cc: Shuah Khan <shuah@...nel.org>
> >> Cc: Catalin Marinas <catalin.marinas@....com>
> >> Cc: Will Deacon <will@...nel.org>
> >> Signed-off-by: Boyan Karatotev <boyan.karatotev@....com>
> >> ---

[...]

> >> diff --git a/tools/testing/selftests/arm64/pauth/pac_corruptor.S b/tools/testing/selftests/arm64/pauth/pac_corruptor.S
> >> new file mode 100644
> >> index 000000000000..6a34ec23a034
> >> --- /dev/null
> >> +++ b/tools/testing/selftests/arm64/pauth/pac_corruptor.S
> >> @@ -0,0 +1,36 @@
> >> +/* SPDX-License-Identifier: GPL-2.0 */
> >> +/* Copyright (C) 2020 ARM Limited */
> >> +
> >> +.global pac_corruptor
> >> +
> >> +.text
> >> +/*
> >> + * Corrupting a single bit of the PAC ensures the authentication will fail.  It
> >> + * also guarantees no possible collision. TCR_EL1.TBI0 is set by default so no
> >> + * top byte PAC is tested
> >> + */
> >> + pac_corruptor:
> >> +	paciasp
> >> +
> >> +	/* make stack frame */
> >> +	sub sp, sp, #16
> >> +	stp x29, lr, [sp]
> > 
> > Nit: if respinning, you can optimise a few sequences of this sort, e.g.
> > 
> > 	stp	x29, lr, [sp, #-16]!
> > 
> >> +	mov x29, sp
> >> +
> >> +	/* prepare mask for bit to be corrupted (bit 54) */
> >> +	mov x1, xzr
> >> +	add x1, x1, #1
> >> +	lsl x1, x1, #54
> > 
> > Nit:
> > 
> > 	mov	x1, #1 << 54
> Thank you for this, didn't know I could do it this way.
> > 
> > but anyway, the logic operations can encode most simple bitmasks
> > directly as immediate operands, so you can skip this and just do
> > 
> >> +
> >> +	/* get saved lr, corrupt selected bit, put it back */
> >> +	ldr x0, [sp, #8]
> >> +	eor x0, x0, x1
> > 
> > 	eor	x0, x0, #1 << 54
> > 
> >> +	str x0, [sp, #8]
> >> +
> >> +	/* remove stack frame */
> >> +	ldp x29, lr, [sp]
> >> +	add sp, sp, #16
> > 
> > 	ldp	x29, lr, [sp], #16
> > 
> > [...]
> > 
> > Actually, since there are no leaf nested function calls and no trap is
> > expected until the function returns (so backtracing in the middle of
> > this function is unlikely to be needed), could we optimise this whole
> > thing down to the following?
> > 
> I suppose you're right. The intent was to emulate a c function but there
> really is no point in doing all this extra work. Will change it.

It's not critical either way, but this way it's at least less code to
maintain / read.

> > pac_corruptor:
> > 	paciasp
> > 	eor	lr, lr, #1 << 53
> > 	autiasp
> > 	ret
> > 
> > Cheers
> > ---Dave

[...]

Cheers
---Dave