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

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



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?

Lin



 


Rackspace

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