linux-kernel - Re: [PATCH v5 01/26] KVM: x86: hyper-v: Expose access to debug MSRs in the partition privilege flags

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: <878rnltw7b.fsf@redhat.com>
Date:   Thu, 18 Aug 2022 17:20:24 +0200
From:   Vitaly Kuznetsov <vkuznets@...hat.com>
To:     Sean Christopherson <seanjc@...gle.com>
Cc:     kvm@...r.kernel.org, Paolo Bonzini <pbonzini@...hat.com>,
        Anirudh Rayabharam <anrayabh@...ux.microsoft.com>,
        Wanpeng Li <wanpengli@...cent.com>,
        Jim Mattson <jmattson@...gle.com>,
        Maxim Levitsky <mlevitsk@...hat.com>,
        Nathan Chancellor <nathan@...nel.org>,
        Michael Kelley <mikelley@...rosoft.com>,
        linux-hyperv@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v5 01/26] KVM: x86: hyper-v: Expose access to debug MSRs
 in the partition privilege flags

Sean Christopherson <seanjc@...gle.com> writes:

> On Tue, Aug 02, 2022, Vitaly Kuznetsov wrote:
>> For some features, Hyper-V spec defines two separate CPUID bits: one
>> listing whether the feature is supported or not and another one showing
>> whether guest partition was granted access to the feature ("partition
>> privilege mask"). 'Debug MSRs available' is one of such features. Add
>> the missing 'access' bit.
>> 
>> Note: hv_check_msr_access() deliberately keeps checking
>> HV_FEATURE_DEBUG_MSRS_AVAILABLE bit instead of the new HV_ACCESS_DEBUG_MSRS
>> to not break existing VMMs (QEMU) which only expose one bit. Normally, VMMs
>> should set either both these bits or none.
>
> This is not the right approach long term.  If KVM absolutely cannot unconditionally
> switch to checking HV_ACCESS_DEBUG_MSRS because it would break QEMU users, then we
> should add a quirk, but sweeping the whole thing under the rug is wrong.
>

First, this patch is kind of unrelated to the series so in case it's the
only thing which blocks it from being merged -- let's just pull it out
and discuss separately.

My personal opinion is that in this particular case we actually can
switch to checking HV_ACCESS_DEBUG_MSRS and possibly backport this patch
to stable@ and be done with it as SynDBG is a debug feature which is not
supposed to be used much in the wild. This, however, will not give us
much besides 'purity' in KVM as no sane VMM is supposed to set just one
of the HV_FEATURE_DEBUG_MSRS_AVAILABLE/HV_ACCESS_DEBUG_MSRS bits. TL;DR:
I'm not against the change.

-- 
Vitaly