|
[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
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |