This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
Home Products Support Community News


Re: [Xen-devel] Unsigned bug in rdmsr_hypervisor_regs/wrmsr_hypervisor_r

To: Robert Phillips <rsp.vi.xen@xxxxxxxxx>, <xen-devel@xxxxxxxxxxxxxxxxxxx>
Subject: Re: [Xen-devel] Unsigned bug in rdmsr_hypervisor_regs/wrmsr_hypervisor_regs
From: Keir Fraser <Keir.Fraser@xxxxxxxxxxxx>
Date: Thu, 27 Sep 2007 15:28:14 +0100
Delivery-date: Thu, 27 Sep 2007 07:29:05 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxx
In-reply-to: <fc060d960709270721r4bc49377w9aff12fd167ca3df@xxxxxxxxxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/cgi-bin/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/cgi-bin/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
Thread-index: AcgBEqNA4b7xy20FEdyhdwAX8io7RQ==
Thread-topic: [Xen-devel] Unsigned bug in rdmsr_hypervisor_regs/wrmsr_hypervisor_regs
User-agent: Microsoft-Entourage/
The code is correct as it is. The function returns 1 if it supplies the requested CPUID leaf, else 0. It only currently supplies leaf 0x40000000, so it is correct to return 0 for all other index values.

 -- Keir

On 27/9/07 15:21, "Robert Phillips" <rsp.vi.xen@xxxxxxxxx> wrote:

The code, below, in rdmsr_hypervisor_regs (in xen/arch/x86/traps.c) looks wrong.  (The same code is in wrmsr_hypervisor_regs.)

int rdmsr_hypervisor_regs(
    uint32_t idx, uint32_t *eax, uint32_t *edx)
    idx -= 0x40000000;
    if ( idx > 0 )
        return 0;

The intent, apparently, is that the function should return zero if the original idx exceeds 0x40000000.
However because idx is unsigned the function will always return zero unless the original idx is precisely 0x40000000.

The effect is that reading or writing any unexpected MSR will cause the guest to get a GPF.
(The intended effect is, I think, reading any MSR less than 0x40000001 should simply return 0
and writing any such MSR should bug.  Injecting a GPF should only happen with MSRs greater than 0x40000000.)

Replacing the test with
   if ( (int32_t)idx > 0 )
 seems to cause problems of its own.

If this change is wrong, what is a proper fix?

-- rsp

Xen-devel mailing list
<Prev in Thread] Current Thread [Next in Thread>