linux-kernel - Re: [PATCH RESEND v3 03/11] KVM: x86/pmu: Protect kvm->arch.pmu_event

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <31f3de9a-a752-e322-ebd0-731c42afd47a@redhat.com>
Date:   Fri, 20 May 2022 14:51:06 +0200
From:   Paolo Bonzini <pbonzini@...hat.com>
To:     Like Xu <like.xu.linux@...il.com>
Cc:     Sean Christopherson <seanjc@...gle.com>,
        Jim Mattson <jmattson@...gle.com>,
        Vitaly Kuznetsov <vkuznets@...hat.com>,
        Wanpeng Li <wanpengli@...cent.com>,
        Joerg Roedel <joro@...tes.org>, linux-kernel@...r.kernel.org,
        kvm@...r.kernel.org
Subject: Re: [PATCH RESEND v3 03/11] KVM: x86/pmu: Protect
 kvm->arch.pmu_event_filter with SRCU

On 5/18/22 15:25, Like Xu wrote:
> From: Like Xu <likexu@...cent.com>
> 
> Similar to "kvm->arch.msr_filter", KVM should guarantee that vCPUs will
> see either the previous filter or the new filter when user space calls
> KVM_SET_PMU_EVENT_FILTER ioctl with the vCPU running so that guest
> pmu events with identical settings in both the old and new filter have
> deterministic behavior.
> 
> Fixes: 66bb8a065f5a ("KVM: x86: PMU Event Filter")
> Signed-off-by: Like Xu <likexu@...cent.com>
> Reviewed-by: Wanpeng Li <wanpengli@...cent.com>

Please always include the call trace where SRCU is not taken.  The ones 
I reconstructed always end up at a place inside srcu_read_lock/unlock:

reprogram_gp_counter/reprogram_fixed_counter
   amd_pmu_set_msr
    kvm_set_msr_common
     svm_set_msr
      __kvm_set_msr
      kvm_set_msr_ignored_check
       kvm_set_msr_with_filter
        kvm_emulate_wrmsr**
        emulator_set_msr_with_filter**
       kvm_set_msr
        emulator_set_msr**
       do_set_msr
        __msr_io
         msr_io
          ioctl(KVM_SET_MSRS)**
   intel_pmu_set_msr
    kvm_set_msr_common
     vmx_set_msr (see svm_set_msr)
   reprogram_counter
    global_ctrl_changed
     intel_pmu_set_msr (see above)
    kvm_pmu_handle_event
     vcpu_enter_guest**
    kvm_pmu_incr_counter
     kvm_pmu_trigger_event
      nested_vmx_run**
      kvm_skip_emulated_instruction**
      x86_emulate_instruction**
   reprogram_fixed_counters
    intel_pmu_set_msr (see above)

Paolo

>   arch/x86/kvm/pmu.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kvm/pmu.c b/arch/x86/kvm/pmu.c
> index f189512207db..24624654e476 100644
> --- a/arch/x86/kvm/pmu.c
> +++ b/arch/x86/kvm/pmu.c
> @@ -246,8 +246,9 @@ static bool check_pmu_event_filter(struct kvm_pmc *pmc)
>   	struct kvm *kvm = pmc->vcpu->kvm;
>   	bool allow_event = true;
>   	__u64 key;
> -	int idx;
> +	int idx, srcu_idx;
>   
> +	srcu_idx = srcu_read_lock(&kvm->srcu);
>   	filter = srcu_dereference(kvm->arch.pmu_event_filter, &kvm->srcu);
>   	if (!filter)
>   		goto out;
> @@ -270,6 +271,7 @@ static bool check_pmu_event_filter(struct kvm_pmc *pmc)
>   	}
>   
>   out:
> +	srcu_read_unlock(&kvm->srcu, srcu_idx);
>   	return allow_event;
>   }
>