From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Jan Beulich" Subject: [PATCH] x86emul: also put_fpu() on error paths Date: Fri, 15 May 2015 13:37:22 +0100 Message-ID: <555604A2020000780007A8CD@mail.emea.novell.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="=__Part12261F92.1__=" Return-path: Received: from mail6.bemta5.messagelabs.com ([195.245.231.135]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1YtErj-0001wC-1t for xen-devel@lists.xenproject.org; Fri, 15 May 2015 12:37:23 +0000 List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: xen-devel Cc: Andrew Cooper , Keir Fraser List-Id: xen-devel@lists.xenproject.org This is a MIME message. If you are reading this text, you may want to consider changing to a mail reader or gateway that understands how to properly handle MIME multipart messages. --=__Part12261F92.1__= Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: quoted-printable Content-Disposition: inline fail_if() and generate_exception_if() could theoretically bypass the normal flow reaching put_fpu(), and not invoking it would leave the fpu_exception_callback pointer in place, allowing for the callback to be called at an unexpected time. Luckily the two generate_exception_if()-s that would actually trigger this are currently commented out, so this is not (yet) a (security) issue. Signed-off-by: Jan Beulich --- a/xen/arch/x86/x86_emulate/x86_emulate.c +++ b/xen/arch/x86/x86_emulate/x86_emulate.c @@ -670,10 +670,14 @@ do{ (_fic)->exn_raised =3D 0; =20 rc =3D ops->get_fpu(fpu_handle_exception, _fic, _type, ctxt); \ if ( rc ) goto done; \ } while (0) -#define put_fpu(_fic) \ -do{ \ +#define _put_fpu() \ +do { \ if ( ops->put_fpu !=3D NULL ) \ - ops->put_fpu(ctxt); \ + (ops->put_fpu)(ctxt); \ +} while (0) +#define put_fpu(_fic) \ +do { \ + _put_fpu(); \ generate_exception_if((_fic)->exn_raised, EXC_MF, -1); \ } while (0) =20 @@ -3787,6 +3791,7 @@ x86_emulate( *ctxt->regs =3D _regs; =20 done: + _put_fpu(); return rc; =20 twobyte_insn: @@ -4632,5 +4637,6 @@ x86_emulate( goto writeback; =20 cannot_emulate: + _put_fpu(); return X86EMUL_UNHANDLEABLE; } --- a/xen/arch/x86/x86_emulate/x86_emulate.h +++ b/xen/arch/x86/x86_emulate/x86_emulate.h @@ -384,7 +384,11 @@ struct x86_emulate_ops enum x86_emulate_fpu_type type, struct x86_emulate_ctxt *ctxt); =20 - /* put_fpu: Relinquish the FPU. Unhook from FPU/SIMD exception = handlers. */ + /* + * put_fpu: Relinquish the FPU. Unhook from FPU/SIMD exception = handlers. + * The handler, if installed, must be prepared to get called without + * the get_fpu one having got called before! + */ void (*put_fpu)( struct x86_emulate_ctxt *ctxt); =20 --=__Part12261F92.1__= Content-Type: text/plain; name="x86emul-put-FPU.patch" Content-Transfer-Encoding: quoted-printable Content-Disposition: attachment; filename="x86emul-put-FPU.patch" x86emul: also put_fpu() on error paths=0A=0Afail_if() and generate_exceptio= n_if() could theoretically bypass the=0Anormal flow reaching put_fpu(), = and not invoking it would leave the=0Afpu_exception_callback pointer in = place, allowing for the callback to=0Abe called at an unexpected time. = Luckily the two=0Agenerate_exception_if()-s that would actually trigger = this are=0Acurrently commented out, so this is not (yet) a (security) = issue.=0A=0ASigned-off-by: Jan Beulich =0A=0A--- = a/xen/arch/x86/x86_emulate/x86_emulate.c=0A+++ b/xen/arch/x86/x86_emulate/x= 86_emulate.c=0A@@ -670,10 +670,14 @@ do{ (_fic)->exn_raised =3D 0; = =0A rc =3D ops->get_fpu(fpu_handle_exception, _fic, _type, ctxt); = \=0A if ( rc ) goto done; \=0A = } while (0)=0A-#define put_fpu(_fic) = \=0A-do{ = \=0A+#define _put_fpu() = \=0A+do { \=0A = if ( ops->put_fpu !=3D NULL ) \=0A- = ops->put_fpu(ctxt); \=0A+ = (ops->put_fpu)(ctxt); \=0A+} while = (0)=0A+#define put_fpu(_fic) = \=0A+do { \=0A+ = _put_fpu(); \=0A = generate_exception_if((_fic)->exn_raised, EXC_MF, -1); \=0A } while = (0)=0A =0A@@ -3787,6 +3791,7 @@ x86_emulate(=0A *ctxt->regs =3D = _regs;=0A =0A done:=0A+ _put_fpu();=0A return rc;=0A =0A = twobyte_insn:=0A@@ -4632,5 +4637,6 @@ x86_emulate(=0A goto writeback;= =0A =0A cannot_emulate:=0A+ _put_fpu();=0A return X86EMUL_UNHANDLEA= BLE;=0A }=0A--- a/xen/arch/x86/x86_emulate/x86_emulate.h=0A+++ b/xen/arch/x= 86/x86_emulate/x86_emulate.h=0A@@ -384,7 +384,11 @@ struct x86_emulate_ops= =0A enum x86_emulate_fpu_type type,=0A struct x86_emulate_c= txt *ctxt);=0A =0A- /* put_fpu: Relinquish the FPU. Unhook from = FPU/SIMD exception handlers. */=0A+ /*=0A+ * put_fpu: Relinquish = the FPU. Unhook from FPU/SIMD exception handlers.=0A+ * The handler, = if installed, must be prepared to get called without=0A+ * the = get_fpu one having got called before!=0A+ */=0A void (*put_fpu)(=0A= struct x86_emulate_ctxt *ctxt);=0A =0A --=__Part12261F92.1__= Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel --=__Part12261F92.1__=--