linux-kernel - Re: [PATCH v4 08/16] KVM: x86/pmu: Add IA32_DS_AREA MSR emulation to manage guest DS buffer

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]

Date:   Wed, 7 Apr 2021 17:39:42 +0200
From:   Peter Zijlstra <peterz@...radead.org>
To:     Like Xu <like.xu@...ux.intel.com>
Cc:     Sean Christopherson <seanjc@...gle.com>,
        Paolo Bonzini <pbonzini@...hat.com>, eranian@...gle.com,
        andi@...stfloor.org, kan.liang@...ux.intel.com,
        wei.w.wang@...el.com, Wanpeng Li <wanpengli@...cent.com>,
        Vitaly Kuznetsov <vkuznets@...hat.com>,
        Jim Mattson <jmattson@...gle.com>,
        Joerg Roedel <joro@...tes.org>, kvm@...r.kernel.org,
        x86@...nel.org, linux-kernel@...r.kernel.org,
        Andi Kleen <ak@...ux.intel.com>
Subject: Re: [PATCH v4 08/16] KVM: x86/pmu: Add IA32_DS_AREA MSR emulation to
 manage guest DS buffer

On Mon, Mar 29, 2021 at 01:41:29PM +0800, Like Xu wrote:
> @@ -3869,10 +3876,12 @@ static struct perf_guest_switch_msr *intel_guest_get_msrs(int *nr, void *data)
>  
>  		if (arr[1].guest)
>  			arr[0].guest |= arr[1].guest;
> -		else
> +		else {
>  			arr[1].guest = arr[1].host;
> +			arr[2].guest = arr[2].host;
> +		}

What's all this gibberish?

The way I read that it says:

	if guest has PEBS_ENABLED
		guest GLOBAL_CTRL |= PEBS_ENABLED
	otherwise
		guest PEBS_ENABLED = host PEBS_ENABLED
		guest DS_AREA = host DS_AREA

which is just completely random garbage afaict. Why would you leak host
msrs into the guest? Why would you change guest GLOBAL_CTRL implicitly;
guest had better wrmsr that himself to control when stuff is enabled.

This just cannot be right.