[Xen-devel] [PATCH v2 5/6] x86emul: avoid undefined behavior when dealing with 10-byte FPU operands

["Jan Beulich" <JBeulich@xxxxxxxx>]

Accessing an 8-byte (or perhaps just 4-byte in the test harness when
built as 32-bit app) field to read/write 10 bytes (leveraging the
successive field) is a latent bug, as the compiler could copy things
around. Use the 32 bytes large SSE/AVX slot instead.

Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
Reviewed-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
---
v2: Re-base over changes to patch 1.
---
The presence of the !op->write checks implies a logical (but not
functional) dependency on the patch making ops->write (and ->cmpxchg)
optional. Without that patch they're just dead code.

--- a/xen/arch/x86/x86_emulate/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate/x86_emulate.c
@@ -3787,15 +3787,19 @@ x86_emulate(
                 dst.bytes = 4;
                 break;
             case 5: /* fld m80fp */
-                if ( (rc = ops->read(ea.mem.seg, ea.mem.off, &src.val,
+                if ( (rc = ops->read(ea.mem.seg, ea.mem.off, mmvalp,
                                      10, ctxt)) != X86EMUL_OKAY )
                     goto done;
-                emulate_fpu_insn_memsrc("fldt", src.val);
+                emulate_fpu_insn_memsrc("fldt", *mmvalp);
                 dst.type = OP_NONE;
                 break;
             case 7: /* fstp m80fp */
-                emulate_fpu_insn_memdst("fstpt", dst.val);
-                dst.bytes = 10;
+                fail_if(!ops->write);
+                emulate_fpu_insn_memdst("fstpt", *mmvalp);
+                if ( (rc = ops->write(ea.mem.seg, ea.mem.off, mmvalp,
+                                      10, ctxt)) != X86EMUL_OKAY )
+                    goto done;
+                dst.type = OP_NONE;
                 break;
             default:
                 generate_exception(EXC_UD);
@@ -4004,10 +4008,10 @@ x86_emulate(
                 dst.bytes = 2;
                 break;
             case 4: /* fbld m80dec */
-                if ( (rc = ops->read(ea.mem.seg, ea.mem.off, &src.val,
+                if ( (rc = ops->read(ea.mem.seg, ea.mem.off, mmvalp,
                                      10, ctxt)) != X86EMUL_OKAY )
                     goto done;
-                emulate_fpu_insn_memsrc("fbld", src.val);
+                emulate_fpu_insn_memsrc("fbld", *mmvalp);
                 dst.type = OP_NONE;
                 break;
             case 5: /* fild m64i */
@@ -4018,8 +4022,12 @@ x86_emulate(
                 dst.type = OP_NONE;
                 break;
             case 6: /* fbstp packed bcd */
-                emulate_fpu_insn_memdst("fbstp", dst.val);
-                dst.bytes = 10;
+                fail_if(!ops->write);
+                emulate_fpu_insn_memdst("fbstp", *mmvalp);
+                if ( (rc = ops->write(ea.mem.seg, ea.mem.off, mmvalp,
+                                      10, ctxt)) != X86EMUL_OKAY )
+                    goto done;
+                dst.type = OP_NONE;
                 break;
             case 7: /* fistp m64i */
                 emulate_fpu_insn_memdst("fistpll", dst.val);

Attachment: x86emul-FPU-10byte.patch
Description: Text document

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel