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

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

Hi 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.

ok sure. It makes sense to write some details for this scenario. Thanks 
for the details.

> 
>> 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?

I don't have any dynamic performance numbers now. I have some static 
numbers from 5.7-rc1 kernel with a simple grep.

Total functions(leaf+non leaf) = 55682(PACIASP instructions).
Total non-leaf functions = 47425.
so leaf functions = 8257(14.8% of total).

Leaf functions used for indirect calls = 1345 (extra "bti c" 
instructions if -mbranch-protection=standard used).
Leaf functions used for direct calls = 6192.

So statically there should be performance improvement with this patch.

Reduction of extra instructions with ptrauth = 14.8%
Reduction of extra instructions with both ptrauth + bti = 12.32%

Cheers,
Amit
> 
> 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