From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755107AbcJ1D3s convert rfc822-to-8bit (ORCPT ); Thu, 27 Oct 2016 23:29:48 -0400 Received: from mx1.redhat.com ([209.132.183.28]:55506 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750823AbcJ1D3r (ORCPT ); Thu, 27 Oct 2016 23:29:47 -0400 From: Bandan Das To: Radim =?utf-8?B?S3LEjW3DocWZ?= Cc: linux-kernel@vger.kernel.org, kvm@vger.kernel.org, Paolo Bonzini , Laszlo Ersek Subject: Re: [PATCH 1/2] KVM: x86: emulate fxsave and fxrstor References: <20161026205014.19801-1-rkrcmar@redhat.com> <20161026205014.19801-2-rkrcmar@redhat.com> <20161027163457.GB11326@potion> Date: Thu, 27 Oct 2016 23:29:42 -0400 In-Reply-To: <20161027163457.GB11326@potion> ("Radim \=\?utf-8\?B\?S3LEjW0\=\?\= \=\?utf-8\?B\?w6HFmSIncw\=\=\?\= message of "Thu, 27 Oct 2016 18:34:57 +0200") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8BIT X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.30]); Fri, 28 Oct 2016 03:29:46 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Radim Krčmář writes: > 2016-10-26 20:17-0400, Bandan Das: >> Radim Krčmář writes: >> ... >>> +static int check_fxsr(struct x86_emulate_ctxt *ctxt) >>> +{ >>> + u32 eax = 1, ebx, ecx = 0, edx; >>> + >>> + ctxt->ops->get_cpuid(ctxt, &eax, &ebx, &ecx, &edx); >>> + if (!(edx & FFL(FXSR))) >>> + return emulate_ud(ctxt); >>> + >>> + if (ctxt->ops->get_cr(ctxt, 0) & (X86_CR0_TS | X86_CR0_EM)) >>> + return emulate_nm(ctxt); >>> + >>> + return X86EMUL_CONTINUE; >>> +} >>> + >>> +/* >>> + * FXSAVE and FXRSTOR have 3 different formats depending on execution mode, >>> + * 1) non-64-bit mode >>> + * 2) 64-bit mode with REX.W prefix >>> + * 3) 64-bit mode without REX.W prefix >>> + * >>> + * Emulation uses (3) for for (1) mode because only the number of XMM registers >>> + * is different. >>> + */ > | [...] >>> + >>> +static int em_fxrstor(struct x86_emulate_ctxt *ctxt) >>> +{ >>> + char fx_state[512] __aligned(16); >>> + int rc; >>> + >>> + rc = check_fxsr(ctxt); >> >> Is this check enough here ? What I mean is that is it possible that the memory >> image that is read from has data in an invalid format/corrupt or is that irrelevant ? > > No, it is not enough, v2 will need testing on bare metal. :) > > Nadav mentioned that MXCSR could thrown #GP when setting bits 16-32. Yeah, this was what I am wondering about. Whether, there are obvious ways for the guest to abuse the path, you know, common emulator code concerns. > And there are actually 4 different formats: 16 bit mode has only 16 bit > FIP, and other 16 bits are reserved, but KVM's fxrstor would be loading > all 32 bits, so the reserved upper 16 should be cleared beforehand. > The structure has a lot of reserved fields, but they should just be > ignored by the CPU. Ok. > Did you notice other problems? Rest looks fine to me even with the #ifdefs that you don't prefer but I will be sure to take another look when you post a v2. > Thanks. > -- > To unsubscribe from this list: send the line "unsubscribe kvm" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html