* Re: [PATCH v2] arm64: Optimize ptrauth by enabling it for non-leaf functions
2020-04-29 10:18 ` Mark Rutland
@ 2020-04-29 16:01 ` Amit Kachhap
2020-04-30 11:00 ` Amit Kachhap
1 sibling, 0 replies; 5+ messages in thread
From: Amit Kachhap @ 2020-04-29 16:01 UTC (permalink / raw)
To: Mark Rutland
Cc: Catalin Marinas, Mark Brown, James Morse, Vincenzo Frascino,
Will Deacon, linux-arm-kernel, Daniel Kiss
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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] arm64: Optimize ptrauth by enabling it for non-leaf functions
2020-04-29 10:18 ` Mark Rutland
2020-04-29 16:01 ` Amit Kachhap
@ 2020-04-30 11:00 ` Amit Kachhap
2020-04-30 11:05 ` Will Deacon
1 sibling, 1 reply; 5+ messages in thread
From: Amit Kachhap @ 2020-04-30 11:00 UTC (permalink / raw)
To: Mark Rutland, Will Deacon
Cc: Catalin Marinas, Mark Brown, James Morse, Vincenzo Frascino,
linux-arm-kernel, Daniel Kiss
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
^ permalink raw reply [flat|nested] 5+ messages in thread