| Message ID | 1583476525-13505-1-git-send-email-amit.kachhap@arm.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | arm64: return address signing | expand |
Hello, Le perjantaina 6. maaliskuuta 2020, 8.35.07 EET Amit Daniel Kachhap a écrit : > 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. Is there any notion what the runtime overhead will be? We tried to estimate PAuth-based return address signing and an attempt of ours to improve it (ArXiv:1912.04145, to be presented at DAC 2020), but we lacked proper hardware.
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? 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? (insert cross-tree headache here) Thanks, James
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. Cheers, Amit Daniel > > (insert cross-tree headache here) > > > Thanks, > > James >
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. //Amit > > Cheers, > Amit Daniel > >> >> (insert cross-tree headache here) >> >> >> Thanks, >> >> James >>
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. I assume you have a way to hide them from userspace, though. Thanks, M.
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. Cheers, Amit > > Thanks, > > M.
[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.
Hi Amit, Marc, On 12/03/2020 15:05, Marc Zyngier wrote: > On 2020-03-12 13:21, Amit Kachhap wrote: >> On 3/12/20 6:17 PM, Marc Zyngier wrote: >>> On 2020-03-12 08:06, Amit Kachhap wrote: >>>> On 3/12/20 12:23 PM, Amit Kachhap wrote: >>>>> On 3/11/20 2:58 PM, James Morse wrote: >>>>>> 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. > 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. Yes, the arch code entry.S changes cover this with ptrauth_keys_install_user. It restores 4 keys, and the generic key. We always need to save/restore all the guest keys (as we do today). But when ptrauth_restore_state restores the host keys, it only needs to restore the one the kernel uses. (possibly using the same macro's so it stays up to date?!) If we return to user-space, the arch code's entry code does the right thing. KVM's user-space peeking at the keys will see the saved values. My original question was more around: do we need to do this now, or can we clean it up in a later kernel version? (and a sanity check that it doesn't lead to a correctness problem) Thanks, James
Hi James, On 2020-03-12 17:26, James Morse wrote: > Hi Amit, Marc, > > On 12/03/2020 15:05, Marc Zyngier wrote: >> On 2020-03-12 13:21, Amit Kachhap wrote: >>> On 3/12/20 6:17 PM, Marc Zyngier wrote: >>>> On 2020-03-12 08:06, Amit Kachhap wrote: >>>>> On 3/12/20 12:23 PM, Amit Kachhap wrote: >>>>>> On 3/11/20 2:58 PM, James Morse wrote: >>>>>>> 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. > >> 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. > > Yes, the arch code entry.S changes cover this with > ptrauth_keys_install_user. It restores > 4 keys, and the generic key. > > > We always need to save/restore all the guest keys (as we do today). > But when ptrauth_restore_state restores the host keys, it only needs > to restore the one > the kernel uses. (possibly using the same macro's so it stays up to > date?!) > > If we return to user-space, the arch code's entry code does the right > thing. > KVM's user-space peeking at the keys will see the saved values. > > > My original question was more around: do we need to do this now, or > can we clean it up in > a later kernel version? > > (and a sanity check that it doesn't lead to a correctness problem) I think what we have now is sane, and doesn't seem to lead to any issue (at least that I can see). We can always optimize this at a later point. Thanks, M.