| Message ID | 1588149371-20310-1-git-send-email-amit.kachhap@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] arm64: Optimize ptrauth by enabling it for non-leaf functions | expand |
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. > 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 >
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 >>
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 >>
On Thu, Apr 30, 2020 at 04:30:57PM +0530, Amit Kachhap wrote: > On 4/29/20 3:48 PM, Mark Rutland wrote: > > 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. Ok, thanks for letting us know. Will
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
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. 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%. 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(-)