Subject: Re: [PATCH v6 00/18] arm64: return address signing

From: Marc Zyngier <[email-masked]>
Date: Thu, 12 Mar 2020 15:05:39 +0000

[Somehow I managed to butcher the subject line. no idea how...]

On 2020-03-12 13:21, Amit Kachhap wrote:
> Hi Marc,
> 
> On 3/12/20 6:17 PM, Marc Zyngier wrote:
>> Hi Amit,
>> 
>> On 2020-03-12 08:06, Amit Kachhap wrote:
>>> Hi James,
>>> 
>>> On 3/12/20 12:23 PM, Amit Kachhap wrote:
>>>> Hi James,
>>>> 
>>>> On 3/11/20 2:58 PM, James Morse wrote:
>>>>> Hi Amit,
>>>>> 
>>>>> (CC: +Marc)
>>>>> 
>>>>> On 3/6/20 6:35 AM, Amit Daniel Kachhap wrote:
>>>>>> This series improves function return address protection for the 
>>>>>> arm64 kernel, by
>>>>>> compiling the kernel with ARMv8.3 Pointer Authentication 
>>>>>> instructions (referred
>>>>>> ptrauth hereafter). This should help protect the kernel against 
>>>>>> attacks using
>>>>>> return-oriented programming.
>>>>> 
>>>>> (as it looks like there may be another version of this:)
>>>>> 
>>>>> Am I right in thinking that after your patch 10 changing
>>>>> cpu_switch_to(), only the A key is live during kernel execution?
>>>> 
>>>> Yes
>>>> 
>>>>> 
>>>>> KVM is still save/restoring 4 extra keys around guest-entry/exit. 
>>>>> As you
>>>>> restore all the keys on return to user-space, is this still 
>>>>> necessary?
>>>> 
>>>> Yes Its a good optimization to skip 4 non-A keys. I was wondering 
>>>> whether to do it in this series or send it separately.
>>> 
>>> I suppose we can only skip non-A keys save/restore for host context. 
>>> If
>>> we skip non-A keys for guest context then guest with old 
>>> implementation
>>> will break. Let me know your opinion.
>> 
>> I don't think you can skip anything as far as the guest is concerned.
>> But being able to skip the B keys (which is what I expect you call the
>> non-A keys) on the host would certainly be useful.
> 
> Thanks for the clarification.
> 
>> 
>> I assume you have a way to hide them from userspace, though.
> 
> You mean hide all the keys from userspace like below,
> 
> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index 3e909b1..29cc74f 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -1023,7 +1023,7 @@ static bool trap_ptrauth(struct kvm_vcpu *vcpu,
>  static unsigned int ptrauth_visibility(const struct kvm_vcpu *vcpu,
>                         const struct sys_reg_desc *rd)
>  {
> -       return vcpu_has_ptrauth(vcpu) ? 0 : REG_HIDDEN_USER | 
> REG_HIDDEN_GUEST;
> +       return vcpu_has_ptrauth(vcpu) ? REG_HIDDEN_USER :
> REG_HIDDEN_USER | REG_HIDDEN_GUEST;
>  }
> 
>  #define __PTRAUTH_KEY(k)
> 
> I don't remember why it was not done this way last time.

No, that's not what I meant. What you're describing is preventing keys
from being exposed to the VMM controlling the guest, and that'd be
pretty bad (you need to be able to save/restore them for migration).

But if KVM doesn't save/restore the host's B-keys in the world switch,
then you must make sure that no host userspace can make use of them,
as they would be the guest's keys.

         M.
-- 
Jazz is not dead. It just smells funny...

_______________________________________________
linux-arm-kernel mailing list
[email-masked]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel