| Message ID | 54004096.2080104@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Aug 29, 2014, at 11:57 AM, Paolo Bonzini <pbonzini@redhat.com> wrote: > Il 29/08/2014 10:52, Nadav Amit ha scritto: >>> Patch looks good, but where is the check that MOD == 3 in the "case >>> RMExt"? Am I just not seeing it? >> >> This seems to be part of the “case GroupDual”. > > GroupDual handles it, but the EXT() macro you're using is exactly what > you want: > > #define RMExt (4<<15) /* Opcode extension in ModRM r/m if mod == 3 */ > > I guess what's missing is > > ------------------ 8< ------------------ > Subject: [PATCH] Check ModRM for RMExt > > Some group7 extensions are only defined for mod==3. Check this and > reject emulation if mod!=3. > > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > > diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c > index 56657b0bb3bb..d472e4d50e3c 100644 > --- a/arch/x86/kvm/emulate.c > +++ b/arch/x86/kvm/emulate.c > @@ -4360,6 +4360,8 @@ done_prefixes: > opcode = opcode.u.gdual->mod012[goffset]; > break; > case RMExt: > + if ((ctxt->modrm >> 6) == 3) > + return EMULATION_FAILED; > goffset = ctxt->modrm & 7; > opcode = opcode.u.group[goffset]; > break; > > What do you think? I don’t know. I am looking at Intel SDM table A-6 (Opcode Extensions for One- and Two-byte Opcodes by Group Number). According to the table, only group 7 needs RMExt, and in this case the “case GroupDual” makes the required checks, in the iteration prior to the “case RMExt”. Therefore this code path, RMExt without GroupDual before it, should never occur. Nonetheless, if you want to avoid future bugs, perhaps it is good. Nadav
Il 29/08/2014 11:12, Nadav Amit ha scritto: > I don’t know. I am looking at Intel SDM table A-6 (Opcode Extensions > for One- and Two-byte Opcodes by Group Number). According to the > table, only group 7 needs RMExt, and in this case the “case > GroupDual” makes the required checks, in the iteration prior to the > “case RMExt”. Therefore this code path, RMExt without GroupDual > before it, should never occur. Nonetheless, if you want to avoid > future bugs, perhaps it is good. Oh, now I understand what you mean. Thanks, Paolo -- 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
diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c index 56657b0bb3bb..d472e4d50e3c 100644 --- a/arch/x86/kvm/emulate.c +++ b/arch/x86/kvm/emulate.c @@ -4360,6 +4360,8 @@ done_prefixes: opcode = opcode.u.gdual->mod012[goffset]; break; case RMExt: + if ((ctxt->modrm >> 6) == 3) + return EMULATION_FAILED; goffset = ctxt->modrm & 7; opcode = opcode.u.group[goffset]; break;