Message ID | 20210118062809.1430920-3-npiggin@gmail.com |
---|---|
State | Accepted |
Headers | show |
Series | a few KVM patches | expand |
Nicholas Piggin <npiggin@gmail.com> writes: > The slbmte instruction is legal in radix mode, including radix guest > mode. This means radix guests can load the SLB with arbitrary data. > > KVM host does not clear the SLB when exiting a guest if it was a > radix guest, which would allow a rogue radix guest to use the SLB as > a side channel to communicate with other guests. > > Fix this by ensuring the SLB is cleared when coming out of a radix > guest. Only the first 4 entries are a concern, because radix guests > always run with LPCR[UPRT]=1, which limits the reach of slbmte. slbia > is not used (except in a non-performance-critical path) because it > can clear cached translations. > > Signed-off-by: Nicholas Piggin <npiggin@gmail.com> Reviewed-by: Fabiano Rosas <farosas@linux.ibm.com> > --- > arch/powerpc/kvm/book3s_hv_rmhandlers.S | 39 ++++++++++++++++++++----- > 1 file changed, 31 insertions(+), 8 deletions(-) > > diff --git a/arch/powerpc/kvm/book3s_hv_rmhandlers.S b/arch/powerpc/kvm/book3s_hv_rmhandlers.S > index d5a9b57ec129..0e1f5bf168a1 100644 > --- a/arch/powerpc/kvm/book3s_hv_rmhandlers.S > +++ b/arch/powerpc/kvm/book3s_hv_rmhandlers.S > @@ -1157,6 +1157,20 @@ EXPORT_SYMBOL_GPL(__kvmhv_vcpu_entry_p9) > mr r4, r3 > b fast_guest_entry_c > guest_exit_short_path: > + /* > + * Malicious or buggy radix guests may have inserted SLB entries > + * (only 0..3 because radix always runs with UPRT=1), so these must > + * be cleared here to avoid side-channels. slbmte is used rather > + * than slbia, as it won't clear cached translations. > + */ > + li r0,0 > + slbmte r0,r0 > + li r4,1 > + slbmte r0,r4 > + li r4,2 > + slbmte r0,r4 > + li r4,3 > + slbmte r0,r4 > > li r0, KVM_GUEST_MODE_NONE > stb r0, HSTATE_IN_GUEST(r13) > @@ -1469,7 +1483,7 @@ guest_exit_cont: /* r9 = vcpu, r12 = trap, r13 = paca */ > lbz r0, KVM_RADIX(r5) > li r5, 0 > cmpwi r0, 0 > - bne 3f /* for radix, save 0 entries */ > + bne 0f /* for radix, save 0 entries */ > lwz r0,VCPU_SLB_NR(r9) /* number of entries in SLB */ > mtctr r0 > li r6,0 > @@ -1490,12 +1504,9 @@ guest_exit_cont: /* r9 = vcpu, r12 = trap, r13 = paca */ > slbmte r0,r0 > slbia > ptesync > -3: stw r5,VCPU_SLB_MAX(r9) > + stw r5,VCPU_SLB_MAX(r9) > > /* load host SLB entries */ > -BEGIN_MMU_FTR_SECTION > - b 0f > -END_MMU_FTR_SECTION_IFSET(MMU_FTR_TYPE_RADIX) > ld r8,PACA_SLBSHADOWPTR(r13) > > .rept SLB_NUM_BOLTED > @@ -1508,7 +1519,17 @@ END_MMU_FTR_SECTION_IFSET(MMU_FTR_TYPE_RADIX) > slbmte r6,r5 > 1: addi r8,r8,16 > .endr > -0: > + b guest_bypass > + > +0: /* Sanitise radix guest SLB, see guest_exit_short_path comment. */ > + li r0,0 > + slbmte r0,r0 > + li r4,1 > + slbmte r0,r4 > + li r4,2 > + slbmte r0,r4 > + li r4,3 > + slbmte r0,r4 > > guest_bypass: > stw r12, STACK_SLOT_TRAP(r1) > @@ -3302,12 +3323,14 @@ END_FTR_SECTION_IFCLR(CPU_FTR_ARCH_300) > mtspr SPRN_CIABR, r0 > mtspr SPRN_DAWRX0, r0 > > + /* Clear hash and radix guest SLB, see guest_exit_short_path comment. */ > + slbmte r0, r0 > + slbia > + > BEGIN_MMU_FTR_SECTION > b 4f > END_MMU_FTR_SECTION_IFSET(MMU_FTR_TYPE_RADIX) > > - slbmte r0, r0 > - slbia > ptesync > ld r8, PACA_SLBSHADOWPTR(r13) > .rept SLB_NUM_BOLTED
On Mon, Jan 18, 2021 at 04:28:07PM +1000, Nicholas Piggin wrote: > The slbmte instruction is legal in radix mode, including radix guest > mode. This means radix guests can load the SLB with arbitrary data. > > KVM host does not clear the SLB when exiting a guest if it was a > radix guest, which would allow a rogue radix guest to use the SLB as > a side channel to communicate with other guests. No, because the code currently clears the SLB when entering a radix guest, which you remove in the next patch. I'm OK with moving the SLB clearing from guest entry to guest exit, I guess, but I don't see that you are in fact fixing anything by doing so. Paul.
Excerpts from Paul Mackerras's message of February 10, 2021 11:28 am: > On Mon, Jan 18, 2021 at 04:28:07PM +1000, Nicholas Piggin wrote: >> The slbmte instruction is legal in radix mode, including radix guest >> mode. This means radix guests can load the SLB with arbitrary data. >> >> KVM host does not clear the SLB when exiting a guest if it was a >> radix guest, which would allow a rogue radix guest to use the SLB as >> a side channel to communicate with other guests. > > No, because the code currently clears the SLB when entering a radix > guest, Not AFAIKS. > which you remove in the next patch. The next patch avoids clearing host SLB entries when a hash guest is entered from a radix host, it doesn't apply to radix guests. Not sure where the changelog for it went but it should have "HPT guests" in the title at least, I guess. > I'm OK with moving the SLB > clearing from guest entry to guest exit, I guess, but I don't see that > you are in fact fixing anything by doing so. I can set slb entries in a radix guest in simulator and observe they stay around over host<->guest transitions, and they don't after this patch. Thanks, Nick
diff --git a/arch/powerpc/kvm/book3s_hv_rmhandlers.S b/arch/powerpc/kvm/book3s_hv_rmhandlers.S index d5a9b57ec129..0e1f5bf168a1 100644 --- a/arch/powerpc/kvm/book3s_hv_rmhandlers.S +++ b/arch/powerpc/kvm/book3s_hv_rmhandlers.S @@ -1157,6 +1157,20 @@ EXPORT_SYMBOL_GPL(__kvmhv_vcpu_entry_p9) mr r4, r3 b fast_guest_entry_c guest_exit_short_path: + /* + * Malicious or buggy radix guests may have inserted SLB entries + * (only 0..3 because radix always runs with UPRT=1), so these must + * be cleared here to avoid side-channels. slbmte is used rather + * than slbia, as it won't clear cached translations. + */ + li r0,0 + slbmte r0,r0 + li r4,1 + slbmte r0,r4 + li r4,2 + slbmte r0,r4 + li r4,3 + slbmte r0,r4 li r0, KVM_GUEST_MODE_NONE stb r0, HSTATE_IN_GUEST(r13) @@ -1469,7 +1483,7 @@ guest_exit_cont: /* r9 = vcpu, r12 = trap, r13 = paca */ lbz r0, KVM_RADIX(r5) li r5, 0 cmpwi r0, 0 - bne 3f /* for radix, save 0 entries */ + bne 0f /* for radix, save 0 entries */ lwz r0,VCPU_SLB_NR(r9) /* number of entries in SLB */ mtctr r0 li r6,0 @@ -1490,12 +1504,9 @@ guest_exit_cont: /* r9 = vcpu, r12 = trap, r13 = paca */ slbmte r0,r0 slbia ptesync -3: stw r5,VCPU_SLB_MAX(r9) + stw r5,VCPU_SLB_MAX(r9) /* load host SLB entries */ -BEGIN_MMU_FTR_SECTION - b 0f -END_MMU_FTR_SECTION_IFSET(MMU_FTR_TYPE_RADIX) ld r8,PACA_SLBSHADOWPTR(r13) .rept SLB_NUM_BOLTED @@ -1508,7 +1519,17 @@ END_MMU_FTR_SECTION_IFSET(MMU_FTR_TYPE_RADIX) slbmte r6,r5 1: addi r8,r8,16 .endr -0: + b guest_bypass + +0: /* Sanitise radix guest SLB, see guest_exit_short_path comment. */ + li r0,0 + slbmte r0,r0 + li r4,1 + slbmte r0,r4 + li r4,2 + slbmte r0,r4 + li r4,3 + slbmte r0,r4 guest_bypass: stw r12, STACK_SLOT_TRAP(r1) @@ -3302,12 +3323,14 @@ END_FTR_SECTION_IFCLR(CPU_FTR_ARCH_300) mtspr SPRN_CIABR, r0 mtspr SPRN_DAWRX0, r0 + /* Clear hash and radix guest SLB, see guest_exit_short_path comment. */ + slbmte r0, r0 + slbia + BEGIN_MMU_FTR_SECTION b 4f END_MMU_FTR_SECTION_IFSET(MMU_FTR_TYPE_RADIX) - slbmte r0, r0 - slbia ptesync ld r8, PACA_SLBSHADOWPTR(r13) .rept SLB_NUM_BOLTED
The slbmte instruction is legal in radix mode, including radix guest mode. This means radix guests can load the SLB with arbitrary data. KVM host does not clear the SLB when exiting a guest if it was a radix guest, which would allow a rogue radix guest to use the SLB as a side channel to communicate with other guests. Fix this by ensuring the SLB is cleared when coming out of a radix guest. Only the first 4 entries are a concern, because radix guests always run with LPCR[UPRT]=1, which limits the reach of slbmte. slbia is not used (except in a non-performance-critical path) because it can clear cached translations. Signed-off-by: Nicholas Piggin <npiggin@gmail.com> --- arch/powerpc/kvm/book3s_hv_rmhandlers.S | 39 ++++++++++++++++++++----- 1 file changed, 31 insertions(+), 8 deletions(-)