[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: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
- From: Roger Pau Monné <roger.pau@xxxxxxxxxx>
- Date: Wed, 23 Aug 2023 15:31:21 +0200
- 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=4IhcFbDYVzEkZuOzrHZ8RaicR1GnnISMLxAUpr9BDOE=; b=Utes5OKlqQFEFKt1NcJIFDBkuSRxzgIJcBeSi5teFeAXmXqvs/7k+xLmRyVIOZYPB6hlygt5iAZXKAxKOBMzIQpgXGnsvl2nxJuJDYPPff+/nNWi7NFA6nGbxjww4uiIQJRPvtNtfJcDN/HKocyDqiWprOOHvO5SsUG1LoW4nmOV6EJPIQHeIPr9ZB71ot5Ohm6Vcwukof/2w4gaoxsWVnr9WXoexSucL2PJfH1995Hf9j+RdANMDAKfNTvEAhnsH9hW9y0xI0bD/9T+sktdXXFsxsfLxqfChre/ZeBI0FBG3HMf5jGMK8onJBVVEvDlFY/iZBLz4pQHS3Cmbj72NQ==
- Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=GkTDPLz54CEy7/VZfoo6jpugBB4kIIgfLVlOE/HUjpzUiHUEeiUmt5pUFVxoNIZXa0saTKhlHies/CkZiU3+X1BY4hrzNw7X2QTTpvUDGkzmasZMUeBBVGLKdJh07GhDRiF59fP1+w0rfJgX+Kim38dkrLUtnwHtpf7cQxPFlZ4/G6K9v5HIyTYyVDPYMmiceLwXVNkvHWwWEQyDd6DeCS4zTEKUw2idrJCDU3n6L69/orlcj3lppT8cGLK0P67F5g5XQPqT1GqyyZuhts0DZ6hm5ETWl0qz2+SWtotJ7F8OxpaOofCpSmC0uSrdapMBS0A20iax4jb3vMnTSOuerw==
- 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 13:31:47 +0000
- Ironport-data: A9a23:Rz2B1qPhCrAursvvrR1xlsFynXyQoLVcMsEvi/4bfWQNrUomgzcAm GEWX2mOaPeNNjDxKN4kao2+9UhQ7JTdm9NqGQto+SlhQUwRpJueD7x1DKtS0wC6dZSfER09v 63yTvGacajYm1eF/k/F3oDJ9CU6jufQAOKnUoYoAwgpLSd8UiAtlBl/rOAwh49skLCRDhiE/ Nj/uKUzAnf8s9JPGjxSs/vrRC9H5qyo42tI5AdmPJingXeF/5UrJMNHTU2OByOQrrl8RoaSW +vFxbelyWLVlz9F5gSNy+uTnuUiG9Y+DCDW4pZkc/HKbitq/0Te5p0TJvsEAXq7vh3S9zxHJ HehgrTrIeshFvWkdO3wyHC0GQkmVUFN0OevzXRSLaV/ZqAJGpfh66wGMa04AWEX0rdQEFFq/ +JDFBMqVSiSuLzt8vWqY+Y506zPLOGzVG8ekldJ6GiBSNoDH9XESaiM4sJE1jAtgMwIBezZe 8cSdTtoalLHfgFLPVAUTpk5mY9EhFGmK2Ee9A3T+PpxujCMpOBy+OGF3N79YNuFSN8Thk+Fj mnH4374ElcRM9n3JT+tqyj02reTzX+kMG4UPJaD0e5lpVGd/2w4KEAqSkahn+ei2kHrDrqzL GRRoELCt5Ma5EGtC9XwQRC8iHqFpQIHHcpdFfUg7wOAwbaS5ByWbkAmZDNcbN0ttOctWCcnk FSOmrvBGjhHoLCTD3WH+d+8ry60fy4cLmYAZCoNZQoD/9Tn5oo0i3rnTdt9Eajzktz8Hxnxx SyHqG41gLB7sCIQ/6Cy/FSCjzfyoJHMF1Yx/l+OBj3j6R5lbom4YYDu8ULc8ftLMIeeSB+Go WQAnM+dqusJCPlhiRCwfQnEJ5nxj97tDdEWqQcH80UJn9h1x0OeQA==
- Ironport-hdrordr: A9a23:8HjxgqoeBlbvGAN5RMrBpLkaV5rReYIsimQD101hICG9Evb0qy lhppQmPH7P+VIssRQb8+xoV5PufZqxz/BICOoqTNKftWvdyQiVxehZhOOP/9SJIUbDH4VmpM VdmsZFaeEZDTJB/LvHCAvTKadd/DFQmprY+ts3zB1WPH9Xg7kL1XYfNu4CeHcGPzWvA/ACZf yhz/sCnRWMU1INYP+2A3EUNtKz3eEixPrdEGc77wdM0nj3sQ+V
- List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
On Wed, Aug 23, 2023 at 12:56:48PM +0100, Andrew Cooper wrote:
> 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.
Right, but taking this patch without also having 81d3a0b26c1 will lead
to a vulnerable system, hence why I think the dependency would better
be on 81d3a0b26c1.
Anyway, I don't think it's worth arguing over, so if you want to leave
it as-is I won't object.
Thanks, Roger.
|
