| Message ID | 1586856741-26839-1-git-send-email-amit.kachhap@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | arm64: Optimize ptrauth by enabling it for non-leaf functions | expand |
On Tue, Apr 14, 2020 at 03:02:21PM +0530, Amit Daniel Kachhap wrote: > Compilers are optimized to not store the stack frame record for the leaf > function in the stack so applying pointer authentication in the leaf > function is not useful from security point of view. I'm missing the reasoning here -- why don't we care about leaf functions? Sounds like there's a performance/security trade-off that needs spelling out and justifying with some numbers, or is it clear-cut and I'm missing something? Will
On Tue, Apr 14, 2020 at 11:00:33AM +0100, Will Deacon wrote: > On Tue, Apr 14, 2020 at 03:02:21PM +0530, Amit Daniel Kachhap wrote: > > Compilers are optimized to not store the stack frame record for the leaf > > function in the stack so applying pointer authentication in the leaf > > function is not useful from security point of view. > > I'm missing the reasoning here -- why don't we care about leaf functions? > > Sounds like there's a performance/security trade-off that needs spelling > out and justifying with some numbers, or is it clear-cut and I'm missing > something? I believe this is because leaf functions don't store the LR to the stack (as they don't create a frame record), so it cannot be modified by a stray memory write. Amit, if you create a leaf function like: void leaf_function(void) { asm volatile("" : : : "x30"); } ... what assembly does the compiler generate when passed `-msign-return-address=non-leaf` ? * Does the compiler create a stack-frame for this function? * Where does the compiler spill x30? * Does the compiler sign the LR? Thanks, Mark.
Hi, On 4/14/20 3:30 PM, Will Deacon wrote: > On Tue, Apr 14, 2020 at 03:02:21PM +0530, Amit Daniel Kachhap wrote: >> Compilers are optimized to not store the stack frame record for the leaf >> function in the stack so applying pointer authentication in the leaf >> function is not useful from security point of view. > > I'm missing the reasoning here -- why don't we care about leaf functions? > > Sounds like there's a performance/security trade-off that needs spelling > out and justifying with some numbers, or is it clear-cut and I'm missing > something? Since lr is not stored on the stack so this function cannot be used for ROP gadget attack. Sure this also provides performance benefit. I will provide the percentage of code size reduction. Cheers, Amit > > Will >
Hi, On 4/14/20 3:46 PM, Mark Rutland wrote: > On Tue, Apr 14, 2020 at 11:00:33AM +0100, Will Deacon wrote: >> On Tue, Apr 14, 2020 at 03:02:21PM +0530, Amit Daniel Kachhap wrote: >>> Compilers are optimized to not store the stack frame record for the leaf >>> function in the stack so applying pointer authentication in the leaf >>> function is not useful from security point of view. >> >> I'm missing the reasoning here -- why don't we care about leaf functions? >> >> Sounds like there's a performance/security trade-off that needs spelling >> out and justifying with some numbers, or is it clear-cut and I'm missing >> something? > > I believe this is because leaf functions don't store the LR to the stack > (as they don't create a frame record), so it cannot be modified by a > stray memory write. > > Amit, if you create a leaf function like: > > void leaf_function(void) > { > asm volatile("" : : : "x30"); > } > > ... what assembly does the compiler generate when passed > `-msign-return-address=non-leaf` ? This is the assembly generated in this case, 80: d503233f paciasp 84: a9bf7bfd stp x29, x30, [sp, #-16]! 88: 910003fd mov x29, sp asm volatile("" : : : "x30"); } 8c: a8c17bfd ldp x29, x30, [sp], #16 90: d50323bf autiasp 94: d65f03c0 ret > > * Does the compiler create a stack-frame for this function? > * Where does the compiler spill x30? > * Does the compiler sign the LR? Yes here the compiler creates stack frame and pushes signed lr in the stack. Probably this situation with x30 as clobber registers breaks this patch. Cheers, Amit > > Thanks, > Mark. >
On Tue, Apr 14, 2020 at 11:16:49AM +0100, Mark Rutland wrote: > On Tue, Apr 14, 2020 at 11:00:33AM +0100, Will Deacon wrote: > > On Tue, Apr 14, 2020 at 03:02:21PM +0530, Amit Daniel Kachhap wrote: > > > Compilers are optimized to not store the stack frame record for the leaf > > > function in the stack so applying pointer authentication in the leaf > > > function is not useful from security point of view. > > > > I'm missing the reasoning here -- why don't we care about leaf functions? > > > > Sounds like there's a performance/security trade-off that needs spelling > > out and justifying with some numbers, or is it clear-cut and I'm missing > > something? > > I believe this is because leaf functions don't store the LR to the stack > (as they don't create a frame record), so it cannot be modified by a > stray memory write. That makes some sense, but doesn't it also mean you can jump into the middle of a leaf function and it will happily return to whatever sits in LR? Perhaps it would make sense to relax to the 'non-leaf' version only if stack protector is enabled? Will
On Tue, Apr 14, 2020 at 12:00:56PM +0100, Will Deacon wrote: > On Tue, Apr 14, 2020 at 11:16:49AM +0100, Mark Rutland wrote: > > On Tue, Apr 14, 2020 at 11:00:33AM +0100, Will Deacon wrote: > > > On Tue, Apr 14, 2020 at 03:02:21PM +0530, Amit Daniel Kachhap wrote: > > > > Compilers are optimized to not store the stack frame record for the leaf > > > > function in the stack so applying pointer authentication in the leaf > > > > function is not useful from security point of view. > > > > > > I'm missing the reasoning here -- why don't we care about leaf functions? > > > > > > Sounds like there's a performance/security trade-off that needs spelling > > > out and justifying with some numbers, or is it clear-cut and I'm missing > > > something? > > > > I believe this is because leaf functions don't store the LR to the stack > > (as they don't create a frame record), so it cannot be modified by a > > stray memory write. > > That makes some sense, but doesn't it also mean you can jump into the middle > of a leaf function and it will happily return to whatever sits in LR? If you can do that, you've already subverted control flow, and can probably do the same for a regular function, since for: | AUTIASP | RET ... you can just jump to the RET instead. I agree that with RETAA/RETAB this would be different. > Perhaps it would make sense to relax to the 'non-leaf' version only if > stack protector is enabled? I'm not sure I follow the rationale for that? What does stack protector help with for leaf functions? Thanks, Mark.
On Tue, Apr 14, 2020 at 04:28:04PM +0530, Amit Kachhap wrote: > > Hi, > > On 4/14/20 3:46 PM, Mark Rutland wrote: > > On Tue, Apr 14, 2020 at 11:00:33AM +0100, Will Deacon wrote: > > > On Tue, Apr 14, 2020 at 03:02:21PM +0530, Amit Daniel Kachhap wrote: > > > > Compilers are optimized to not store the stack frame record for the leaf > > > > function in the stack so applying pointer authentication in the leaf > > > > function is not useful from security point of view. > > > > > > I'm missing the reasoning here -- why don't we care about leaf functions? > > > > > > Sounds like there's a performance/security trade-off that needs spelling > > > out and justifying with some numbers, or is it clear-cut and I'm missing > > > something? > > > > I believe this is because leaf functions don't store the LR to the stack > > (as they don't create a frame record), so it cannot be modified by a > > stray memory write. > > > > Amit, if you create a leaf function like: > > > > void leaf_function(void) > > { > > asm volatile("" : : : "x30"); > > } > > > > ... what assembly does the compiler generate when passed > > `-msign-return-address=non-leaf` ? > > This is the assembly generated in this case, > 80: d503233f paciasp > 84: a9bf7bfd stp x29, x30, [sp, #-16]! > 88: 910003fd mov x29, sp > asm volatile("" : : : "x30"); > } > 8c: a8c17bfd ldp x29, x30, [sp], #16 > 90: d50323bf autiasp > 94: d65f03c0 ret > > > > > > * Does the compiler create a stack-frame for this function? > > * Where does the compiler spill x30? > > * Does the compiler sign the LR? > > Yes here the compiler creates stack frame and pushes signed lr in the stack. Thanks for testing that! That looks fine. My concern was that the compiler might spill the register without signing it, but that's evidently not the case. So 'leaf' is a misnomer here, and this is really about functions without frame records, but that's fine. Thanks, Mark.
On Tue, Apr 14, 2020 at 12:09:22PM +0100, Mark Rutland wrote: > On Tue, Apr 14, 2020 at 12:00:56PM +0100, Will Deacon wrote: > > On Tue, Apr 14, 2020 at 11:16:49AM +0100, Mark Rutland wrote: > > > I believe this is because leaf functions don't store the LR to the stack > > > (as they don't create a frame record), so it cannot be modified by a > > > stray memory write. > > > > That makes some sense, but doesn't it also mean you can jump into the middle > > of a leaf function and it will happily return to whatever sits in LR? > > If you can do that, you've already subverted control flow, and can > probably do the same for a regular function, since for: > > | AUTIASP > | RET > > ... you can just jump to the RET instead. Perhaps, but it's not at all clear to me that being able to jump over the AUT instruction is just as easy or useful as being able to jump into the middle of a leaf function, which might act as a form of gadget. The commit message is quite bold in saying "[this] is not useful from security point of view". How would this interact with BTI? Would we need to have different landing pads for leaf functions? > > Perhaps it would make sense to relax to the 'non-leaf' version only if > > stack protector is enabled? > > I'm not sure I follow the rationale for that? What does stack protector > help with for leaf functions? Yeah, course it doesn't help because we're not pushing a frame. Ignore me. Will
On Tue, Apr 14, 2020 at 02:10:06PM +0100, Will Deacon wrote: > On Tue, Apr 14, 2020 at 12:09:22PM +0100, Mark Rutland wrote: > > On Tue, Apr 14, 2020 at 12:00:56PM +0100, Will Deacon wrote: > > > On Tue, Apr 14, 2020 at 11:16:49AM +0100, Mark Rutland wrote: > > > > I believe this is because leaf functions don't store the LR to the stack > > > > (as they don't create a frame record), so it cannot be modified by a > > > > stray memory write. > > > > > > That makes some sense, but doesn't it also mean you can jump into the middle > > > of a leaf function and it will happily return to whatever sits in LR? > > > > If you can do that, you've already subverted control flow, and can > > probably do the same for a regular function, since for: > > > > | AUTIASP > > | RET > > > > ... you can just jump to the RET instead. > > Perhaps, but it's not at all clear to me that being able to jump over the > AUT instruction is just as easy or useful as being able to jump into the > middle of a leaf function, which might act as a form of gadget. The commit > message is quite bold in saying "[this] is not useful from security point > of view". Ah, I see. You're right that this would give some number of potentially useful gadgets. > How would this interact with BTI? Would we need to have different landing > pads for leaf functions? IIRC the compiler would emit a BTI instruction where the PACIASP would have been, unless the function were only ever called directly in which case the BTI can also be omitted. For functions that can only be called directly, this prevents the whole function (which might not be AAPCS compliant) from being a gadget. For functions that can be called indirectly, the only saving is the omission of AUTIASP, which I suspect is not a significant saving. The tradeoff isn't clear to me. Thanks, Mark.
Hi Will/Mark, On 4/14/20 7:37 PM, Mark Rutland wrote: > On Tue, Apr 14, 2020 at 02:10:06PM +0100, Will Deacon wrote: >> On Tue, Apr 14, 2020 at 12:09:22PM +0100, Mark Rutland wrote: >>> On Tue, Apr 14, 2020 at 12:00:56PM +0100, Will Deacon wrote: >>>> On Tue, Apr 14, 2020 at 11:16:49AM +0100, Mark Rutland wrote: >>>>> I believe this is because leaf functions don't store the LR to the stack >>>>> (as they don't create a frame record), so it cannot be modified by a >>>>> stray memory write. >>>> >>>> That makes some sense, but doesn't it also mean you can jump into the middle >>>> of a leaf function and it will happily return to whatever sits in LR? >>> >>> If you can do that, you've already subverted control flow, and can >>> probably do the same for a regular function, since for: >>> >>> | AUTIASP >>> | RET >>> >>> ... you can just jump to the RET instead. >> >> Perhaps, but it's not at all clear to me that being able to jump over the >> AUT instruction is just as easy or useful as being able to jump into the >> middle of a leaf function, which might act as a form of gadget. The commit >> message is quite bold in saying "[this] is not useful from security point >> of view". I re-worded the commit log and posted the V2 version. Cheers, Amit > > Ah, I see. > > You're right that this would give some number of potentially useful > gadgets. > >> How would this interact with BTI? Would we need to have different landing >> pads for leaf functions? > > IIRC the compiler would emit a BTI instruction where the PACIASP would > have been, unless the function were only ever called directly in which > case the BTI can also be omitted. > > For functions that can only be called directly, this prevents the whole > function (which might not be AAPCS compliant) from being a gadget. For > functions that can be called indirectly, the only saving is the omission > of AUTIASP, which I suspect is not a significant saving. > > The tradeoff isn't clear to me. > > Thanks, > Mark. >
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 store the stack frame record for the leaf function in the stack so applying pointer authentication in the leaf function is not useful from security point of view. This patch changes compiler option to -mbranch-protection=pac-ret and -msign-return-address=non-leaf. Reported-by: Daniel Kiss <daniel.kiss@arm.com> Signed-off-by: Amit Daniel Kachhap <amit.kachhap@arm.com> --- arch/arm64/Kconfig | 4 ++-- arch/arm64/Makefile | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-)