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

Re: [PATCH] x86/vmx: Revert "x86/VMX: sanitize rIP before re-entering guest"


  • To: Roger Pau Monné <roger.pau@xxxxxxxxxx>
  • From: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
  • Date: Wed, 23 Aug 2023 12:56:48 +0100
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=citrix.com; dmarc=pass action=none header.from=citrix.com; dkim=pass header.d=citrix.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=Z0Wug4dijtKcrLO1XSoqljFh6bVixogOAiEtJ8eHht8=; b=k8qKI8Eg+iWKh21hjXfFYUac6LEB6e92MFmGB3rzoQEqghUEuvHuBfP+37kNA0FWDC6R+RKsEVYc08j8xrpt7FP226EHVh1BP3KJF1dsUmP9xYgPEQ/ZL9uuI+QCWB6v919ndzddsdbR39U8f4Me8dTaAMyh5h2GdSiofAogz7ajqmKmDt8DlQJ4Rzsw2kWoNhMC/DGpSAP/0Z2kYx31jjiJtRSefHXG3XRxkOGzhIJEY7F6W7uPOBwtSQFXNdlVLV6xg9a+ESBQ0YIyUNxmPe+FBWq0RScaFzwGS0aD71I/SlAh2CMOKO7WO64Ey6ZJT78Fni61yWZgLkaT3hLZ9w==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=Df9h5AtUEKrQz+/FkAf9nUlAhbaIJqPlibRlHByBcHU5odc946klmtRjHhIm//3FoBf0avlBWomDl2RHlRB9Aab/7KvKTqfYLI0kqXurqeSlp1bxQPdLIg02PF5aK1zmwDPk9TTT+eddoRvGEJ01kWS41q2qdhr59EmzXIgcrPNGm9tmrqscGY8TVVHMGe+bn0aPuk5+7X+vS6l0V3ziMd4E8YtdlpnQFP7AlVGbXLwZijWiKa2WMRpmQl7TOoOVhzgny1zZsiwl6Tr0Wl53z950fsKz4YNcovI4YoFZih2X5vEKHuSJAIcAK9jrZv7nd4pdYSHBlTOnMxJHRbJx3Q==
  • Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=citrix.com;
  • Cc: Xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxxx>, Jan Beulich <JBeulich@xxxxxxxx>, Wei Liu <wl@xxxxxxx>, Jun Nakajima <jun.nakajima@xxxxxxxxx>, Kevin Tian <kevin.tian@xxxxxxxxx>
  • Delivery-date: Wed, 23 Aug 2023 11:57:23 +0000
  • Ironport-data: A9a23:lDqlEKKpDAZOWjSkFE+RApQlxSXFcZb7ZxGr2PjKsXjdYENS0zRSm jAaXjvVb62Ka2X9fo1/ad6+/UtQvp6GzoAxTQVlqX01Q3x08seUXt7xwmUcnc+xBpaaEB84t ZV2hv3odp1coqr0/0/1WlTZhSAgk/rOHvykU7Ss1hlZHWdMUD0mhQ9oh9k3i4tphcnRKw6Ws Jb5rta31GWNglaYCUpKrfrawP9TlK6q4mhA7gZlPakjUGL2zBH5MrpOfcldEFOgKmVkNrbSb /rOyri/4lTY838FYj9yuu+mGqGiaue60Tmm0hK6aYD76vRxjnVaPpIAHOgdcS9qZwChxLid/ jnvWauYEm/FNoWU8AgUvoIx/ytWZcWq85efSZSzXFD6I+QrvBIAzt03ZHzaM7H09c5oHEJp1 d0HKAwPRT7drbuu2pWFVOlj05FLwMnDZOvzu1lG5BSAVbMKZM6GRK/Ho9hFwD03m8ZCW+7EY NYUYiZuaxKGZABTPlAQC9Q1m+LAanvXKmUE7g7K4/dnpTGLlmSd05C0WDbRUvWMSd9YgQCzo WXe8n6iKhobKMae2XyO9XfEaurnxHmjAt9ISubknhJsqEGy+0E9Ej5Gb3XlhqeipGS+cuNnN ENBr0LCqoB3riRHVOLVTxC+5XKJoBMYc95RCPEhrhGAzLLO5ASUDXRCSSROAPQEnsIrQT0h1 neSgsjkQzdotdW9Vna15rqS6zSoNkA9L3IEIykNTgIH4tzqiIA1kh/LCN1kFcadjdDrGDe23 zGDqgA/gakeiYgA0KDTwLzcqzelp5yMQgtr4AzSBzqh9lkgPNTjYJG041/G6/oGNJyeUlSKo HkDnY6Z8fwKCpaO0ieKRY3hAY2U2hpMCxWE6XYHInXr327FF6KLFWyI3AxDGQ==
  • Ironport-hdrordr: A9a23:cgB4uq3BoCBBhab9BoMVLwqjBfdyeYIsimQD101hICG9E/bo4v xG+c5xuyMc5wxwZJheo6H9BEDtexLhHP1OkPos1MmZLWvbUQKTRekJ0WKI+UyCJ8SRzJ856U 9qG5IOd+EZZTJB4foTi2ODfOrJD7O8nZyAtKPm6zNIcCkvUqdn6m5Ce3Sm+o8dfng5OXL8fq DslvauYlCbCAUqh7+Adx04dtmGncTPiJXlJTYeHnccmXCzpALt0qf+Dx+bmjwDUzZDqI1SjF TtokjC/6C+tPP+7RfZ2wbonvNrseqk8MJHGMuPzu4KLTn24zzYArhJavm5pTUop+Pq0nYG+e O82ysIDoBI8nbMeWPwmxf3xAX69z4r5xbZuCSlqEqmm9X9WDU5T/VMnphYdByx0TtbgO1B
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On 23/08/2023 12:15 pm, Roger Pau Monné wrote:
> On Wed, Apr 05, 2023 at 10:52:45PM +0100, Andrew Cooper wrote:
>> At the time of XSA-170, the x86 instruction emulator was genuinely broken.  
>> It
>> would load arbitrary values into %rip and putting a check here probably was
>> the best stopgap security fix.  It should have been reverted following c/s
>> 81d3a0b26c1 "x86emul: limit-check branch targets" which corrected the 
>> emulator
>> behaviour.
>>
>> However, everyone involved in XSA-170, myself included, failed to read the 
>> SDM
>> correctly.  On the subject of %rip consistency checks, the SDM stated:
>>
>>   If the processor supports N < 64 linear-address bits, bits 63:N must be
>>   identical
>>
>> A non-canonical %rip (and SSP more recently) is an explicitly legal state in
>> x86, and the VMEntry consistency checks are intentionally off-by-one from a
>> regular canonical check.
>>
>> The consequence of this bug is that Xen will currently take a legal x86 state
>> which would successfully VMEnter, and corrupt it into having 
>> non-architectural
>> behaviour.
>>
>> Furthermore, in the time this bugfix has been pending in public, I
>> successfully persuaded Intel to clarify the SDM, adding the following
>> clarification:
>>
>>   The guest RIP value is not required to be canonical; the value of bit N-1
>>   may differ from that of bit N.
>>
>> Fixes: ffbbfda377 ("x86/VMX: sanitize rIP before re-entering guest")
> I think the fixes tag should likely be "x86emul: limit-check branch
> targets", since it's that commit that missed the revert done here?

Well, not really.  ffbbfda377 really does have a bug, irrespective of
the changes in the emulator.

The presence of 81d3a0b26c1 is why this bugfix is a full revert of
ffbbfda377, and not just an off-by-1 adjustment.

>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> Acked-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>

Thanks.

~Andrew



 


Rackspace

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