[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] x86/nSVM: Validate the L1 MSRPM physical address range



On 16.09.2026 05:19, Lin Liu wrote:
> On 10.09.2026 09:29, Jan Beulich wrote:
>> On 10.09.2026 08:18, Lin Liu wrote:
>>> 8c36d5a500 ("x86/nSVM: Validate the L1 IOPM physical address range") added
>>> this check for the IOPM.  The MSRPM requires a check as well.
>>
>> See https://lists.xen.org/archives/html/xen-devel/2026-08/msg00242.html
>> and Abdelkareem's reply. If you disagree, please go into further detail
>> here.
> 
> I had missed that thread; the patch as posted should not go in as it
> stands.
> 
> The one thing I would still raise is that hvm_copy_from_guest_phys()
> does not only reject an out-of-range MSRPM.  It also rejects one which
> is in range but simply not backed by memory - and hardware accepts
> that.
> 
> I checked on an EPYC 9255: I patched Xen so an ordinary non-nested
> guest's own VMCB carried _msrpm_base_pa = 1 << 45, in range with
> nothing behind it.  The guest ran a full Linux boot; a rejected VMRUN
> would have executed no guest instructions at all.  On an EPYC 9965 I
> then counted MSR exits, and the CPU reads such a map as all ones -
> every one of the ten bits construct_vmcb() clears trapped, against
> none in a control guest on the same boot.  (What an unbacked
> machine-physical read returns is chipset behaviour, not architectural.)
> 
> So what I would like to send as v2 is two patches:
> 
>   1/2  make the MAXPHYADDR range check explicit, as 8c36d5a500 did for
>        the IOPM.  No functional change on its own.
>   2/2  stop a failed copy returning NSVM_ERROR_VVMCB; fill the cached
>        map instead - all ones where L1 set MSR_PROT, zero where it did
>        not.
> 
> They have to go together: that failing copy is currently the only
> thing rejecting an out-of-range MSRPM, so 2/2 alone would leave one
> accepted.
> 
> Does that sound reasonable to you?

On the surface it sounds plausible. I'm not sure we'd be doing ourselves
a favor, though. Non-RAM is rejected in other situations as well, where
in principle the same behavior you describe could apply. Hence imo if we
wanted to go that route, I think we'd want to be consistent. That would
be quite a bit more work.

Andrew - do you have any thoughts here?

Jan



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.