Re: [PATCH v2] arm64: Optimize ptrauth by enabling it for non-leaf functions

[Amit Kachhap <amit.kachhap@arm.com>]

Hi Will/Mark

On 4/29/20 3:48 PM, Mark Rutland wrote:
> Hi Amit,
> 
> On Wed, Apr 29, 2020 at 02:06:10PM +0530, Amit Daniel Kachhap wrote:
>> Compilers are optimized to not create the frame record for the leaf
>> function and hence lr is not signed and stored in the stack. Thus the leaf
>> functions cannot be used for ROP gadget attack.
> 
> IIUC Will's point on the last posting was that leaf functions can be
> used as a restricted ROP gadget, where the LR isn't controlled via the
> stack.
> 
> e.g. you might have a gadget that does something like:
> 
> <gadget>:
> 	LDP	x0, x1, [SP], #16
> 	STR	x0, [x1]
> 	RET				// LR == <gadget>
> 
> ... and if the LR had previously been set up to point to gadget, it
> would return recursively, performing a sequence of arbitrary stores.
> With an AUT, it would fail after the first store.
> 
> That does rely on already subverting control flow (probably via a
> forward-edge BR where we don't have BTI), and so maybe we've already
> lost in practical terms, but there is at least some possibility of a
> gadget that AUT would catch here. There's some nuance to capture in the
> commit message for that.

I had some offline discussion with Daniel Kiss about this patch. I am 
stopping this patch work now as there are some use case of ptrauth 
instructions in leaf functions. This may be re-visited later with 
precise runtime data if needed.

Thanks,
Amit Daniel
> 
>> This patch selects pointer authentication only for non-leaf function
>> and the compiler option is modified to -mbranch-protection=pac-ret and
>> -msign-return-address=non-leaf.
>>
>> As there are no PAC instructions(PACIASP and AUTIASP) inserted in the leaf
>> functions so the kernel code size reduces by ~0.01%.
> 
> Do we expect this to matter? The size difference isn't that large, so is
> there a performance issue?
> 
> Are there any leaf functions which we consider critical to performance?
> 
> I know that one concern is that PACIASP acts as an implicit landing pad,
> and so its use everywhere potentially weakens BTI. Do we have any data
> to indicate that would be a concern here? e.g. with and without this,
> how many instances of  PACIASP and BTI *C exist?
> 
> Thanks,
> Mark.
> 
>> Note, As PACIASP instruction is also used for Armv8.5 BTI branching so the
>> compiler may insert BTI instructions in case of leaf functions which are
>> candidate of JOP gadget for the upcoming BTI in-kernel support.
>>
>> Reported-by: Daniel Kiss <daniel.kiss@arm.com>
>> Signed-off-by: Amit Daniel Kachhap <amit.kachhap@arm.com>
>> ---
>> Changes since v1:
>> * Updated the commit logs as per the comments from Will and Mark[1].
>>
>> [1]: https://www.spinics.net/lists/arm-kernel/msg798518.html
>>
>>
>>   arch/arm64/Kconfig  | 4 ++--
>>   arch/arm64/Makefile | 4 ++--
>>   2 files changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
>> index 40fb05d..29cfe05 100644
>> --- a/arch/arm64/Kconfig
>> +++ b/arch/arm64/Kconfig
>> @@ -1541,11 +1541,11 @@ config ARM64_PTR_AUTH
>>   
>>   config CC_HAS_BRANCH_PROT_PAC_RET
>>   	# GCC 9 or later, clang 8 or later
>> -	def_bool $(cc-option,-mbranch-protection=pac-ret+leaf)
>> +	def_bool $(cc-option,-mbranch-protection=pac-ret)
>>   
>>   config CC_HAS_SIGN_RETURN_ADDRESS
>>   	# GCC 7, 8
>> -	def_bool $(cc-option,-msign-return-address=all)
>> +	def_bool $(cc-option,-msign-return-address=non-leaf)
>>   
>>   config AS_HAS_PAC
>>   	def_bool $(as-option,-Wa$(comma)-march=armv8.3-a)
>> diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile
>> index 85e4149..895f506 100644
>> --- a/arch/arm64/Makefile
>> +++ b/arch/arm64/Makefile
>> @@ -70,8 +70,8 @@ endif
>>   branch-prot-flags-y += $(call cc-option,-mbranch-protection=none)
>>   
>>   ifeq ($(CONFIG_ARM64_PTR_AUTH),y)
>> -branch-prot-flags-$(CONFIG_CC_HAS_SIGN_RETURN_ADDRESS) := -msign-return-address=all
>> -branch-prot-flags-$(CONFIG_CC_HAS_BRANCH_PROT_PAC_RET) := -mbranch-protection=pac-ret+leaf
>> +branch-prot-flags-$(CONFIG_CC_HAS_SIGN_RETURN_ADDRESS) := -msign-return-address=non-leaf
>> +branch-prot-flags-$(CONFIG_CC_HAS_BRANCH_PROT_PAC_RET) := -mbranch-protection=pac-ret
>>   # -march=armv8.3-a enables the non-nops instructions for PAC, to avoid the
>>   # compiler to generate them and consequently to break the single image contract
>>   # we pass it only to the assembler. This option is utilized only in case of non
>> -- 
>> 2.7.4
>>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel