Amit Machhiwal <[email protected]> writes: > The virtual-mode HPT hcall handlers (H_ENTER, H_REMOVE, H_BULK_REMOVE, > H_CLEAR_REF, H_CLEAR_MOD) call into book3s_hv_rm_mmu.c, which accesses > memslots via kvm_memslots_raw(). kvm_memslots_raw() uses > rcu_dereference_raw_check() to bypass SRCU lockdep annotation checking. > This is safe in real mode because the entire guest entry/exit is wrapped > in srcu_read_lock/unlock inside kvmppc_run_core() and > kvmhv_run_single_vcpu(). > > However, since commit 6165d5dd99db ("KVM: PPC: Book3S HV: add virtual > mode handlers for HPT hcalls and page faults"), these same handlers are > also executed in virtual mode via kvmppc_pseries_do_hcall(), which runs > after SRCU has already been released on guest exit. > > A concurrent KVM_SET_USER_MEMORY_REGION deletion or move can therefore > call synchronize_srcu_expedited() — which does not wait for this thread — > and then kfree(slot) and vfree(slot->arch.rmap) while the hcall handler > still holds a raw pointer to the memslot. lock_rmap() then writes to > freed vmalloc memory, leading to use-after-free and memory corruption. > Because kvm_memslots_raw() suppresses lockdep checks, this race is > entirely silent. > > Fix this by factoring out the 7 HPT hcall handlers (H_REMOVE, H_ENTER, > H_READ, H_CLEAR_MOD, H_CLEAR_REF, H_PROTECT, H_BULK_REMOVE) into a > helper function, kvmppc_pseries_do_hpt_hcall(), and wrapping its call > site in srcu_read_lock(&kvm->srcu) / srcu_read_unlock(&kvm->srcu, idx). > Targeting only the HPT hcalls avoids wrapping non-HPT hcalls that sleep > (such as H_CONFER, H_REGISTER_VPA, H_PAGE_INIT) or handlers that already > acquire SRCU internally (such as H_RTAS). > > Fixes: 6165d5dd99db ("KVM: PPC: Book3S HV: add virtual mode handlers for HPT > hcalls and page faults") > Cc: [email protected] # v5.14+ > Suggested-by: Ritesh Harjani (IBM) <[email protected]> > Signed-off-by: Amit Machhiwal <[email protected]> > --- > arch/powerpc/kvm/book3s_hv.c | 70 ++++++++++++++++++------------------ > 1 file changed, 35 insertions(+), 35 deletions(-)
I had looked at this path in depth earlier itself. LGTM. Please feel free to add: Reviewed-by: Ritesh Harjani (IBM) <[email protected]>
