linux-kernel - Re: [PATCH 08/28] KVM: nSVM: Use dedicated array of MSRPM offsets to merge L0 and L1 bitmaps

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: <aEFQd8E4RDvLR5tj@intel.com>
Date: Thu, 5 Jun 2025 16:08:23 +0800
From: Chao Gao <chao.gao@...el.com>
To: Sean Christopherson <seanjc@...gle.com>
CC: Paolo Bonzini <pbonzini@...hat.com>, <kvm@...r.kernel.org>,
	<linux-kernel@...r.kernel.org>, Borislav Petkov <bp@...en8.de>, Xin Li
	<xin@...or.com>, Dapeng Mi <dapeng1.mi@...ux.intel.com>
Subject: Re: [PATCH 08/28] KVM: nSVM: Use dedicated array of MSRPM offsets to
 merge L0 and L1 bitmaps

>Me either.  One option would be to use wrapper macros for the interception helpers
>to fill an array at compile time (similar to how kernel exception fixup works),
>but (a) it requires modifications to the linker scripts to generate the arrays,
>(b) is quite confusing/complex, and (c) it doesn't actually solve the problem,
>it just inverts the problem.  Because as above, there are MSRs we *don't* want
>to expose to L2, and so we'd need to add yet more code to filter those out.
>
>And the failure mode for the inverted case would be worse, because if we missed
>an MSR, KVM would incorrectly give L2 access to an MSR.  Whereas with the current
>approach, a missed MSR simply means L2 gets a slower path; but functionally, it's
>fine (and it has to be fine, because KVM can't force L1 to disable interception).

I agree. The risk of breaking functionality should be very low.

>
>> Removing this array and iterating over direct_access_msrs[] directly is an
>> option but it contradicts this series as one of its purposes is to remove
>> direct_access_msrs[].
>
>Using direct_access_msrs[] wouldn't solve the problem either, because nothing
>ensures _that_ array is up-to-date either.
>
>The best idea I have is to add a test that verifies the MSRs that are supposed
>to be passed through actually are passed through.  It's still effectively manual
>checking, but it would require us to screw up twice, i.e. forget to update both
>the array and the test.  The problem is that there's no easy and foolproof way to
>verify that an MSR is passed through in a selftest.
>
>E.g. it would be possible to precisely detect L2 => L0 MSR exits via a BPF program,
>but incorporating a BPF program into a KVM selftest just to detect exits isn't
>something I'm keen on doing (or maintaining).
>
>Using the "exits" stat isn't foolproof due to NMIs (IRQs can be accounted for via
>"irq_exits", and to a lesser extent page faults (especially if shadow paging is
>in use).
>
>If KVM provided an "msr_exits" stats, it would be trivial to verify interception
>via a selftest, but I can't quite convince myself that MSR exits are interesting
>enough to warrant their own stat.

Yes, there doesn't seem to be an easy solution to this problem. Given its low
risk, I think we can live with it for now and revisit the issue if it truely
becomes a recurring problem (i.e., people keep forgetting to update the array).