Messages in this thread | | | From | Andy Lutomirski <> | Date | Mon, 13 Jul 2020 18:10:07 -0700 | Subject | Re: [PATCH v5] x86/umip: Add emulation/spoofing for SLDT and STR instructions |
| |
On Mon, Jul 13, 2020 at 4:45 PM Ricardo Neri <ricardo.neri-calderon@linux.intel.com> wrote: > > On Sat, Jul 11, 2020 at 02:49:54PM -0700, hpa@zytor.com wrote: > > On July 10, 2020 3:45:25 PM PDT, Brendan Shanks <bshanks@codeweavers.com> wrote: > > >Add emulation/spoofing of SLDT and STR for both 32- and 64-bit > > >processes. > > > > > >Wine users have found a small number of Windows apps using SLDT that > > >were crashing when run on UMIP-enabled systems. > > > > > >Reported-by: Andreas Rammhold <andi@notmuch.email> > > >Originally-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com> > > >Signed-off-by: Brendan Shanks <bshanks@codeweavers.com> > > >--- > > > > > >v5: Capitalize instruction names in comments. > > > > > > arch/x86/kernel/umip.c | 40 +++++++++++++++++++++++++++------------- > > > 1 file changed, 27 insertions(+), 13 deletions(-) > > > > > >diff --git a/arch/x86/kernel/umip.c b/arch/x86/kernel/umip.c > > >index 8d5cbe1bbb3b..2c304fd0bb1a 100644 > > >--- a/arch/x86/kernel/umip.c > > >+++ b/arch/x86/kernel/umip.c > > >@@ -45,11 +45,12 @@ > > >* value that, lies close to the top of the kernel memory. The limit for > > >the GDT > > > * and the IDT are set to zero. > > > * > > >- * Given that SLDT and STR are not commonly used in programs that run > > >on WineHQ > > >- * or DOSEMU2, they are not emulated. > > >- * > > >- * The instruction smsw is emulated to return the value that the > > >register CR0 > > >+ * The instruction SMSW is emulated to return the value that the > > >register CR0 > > > * has at boot time as set in the head_32. > > >+ * SLDT and STR are emulated to return the values that the kernel > > >programmatically > > >+ * assigns: > > >+ * - SLDT returns (GDT_ENTRY_LDT * 8) if an LDT has been set, 0 if > > >not. > > >+ * - STR returns (GDT_ENTRY_TSS * 8). > > > * > > > * Emulation is provided for both 32-bit and 64-bit processes. > > > * > > >@@ -244,16 +245,34 @@ static int emulate_umip_insn(struct insn *insn, > > >int umip_inst, > > > *data_size += UMIP_GDT_IDT_LIMIT_SIZE; > > > memcpy(data, &dummy_limit, UMIP_GDT_IDT_LIMIT_SIZE); > > > > > >- } else if (umip_inst == UMIP_INST_SMSW) { > > >- unsigned long dummy_value = CR0_STATE; > > >+ } else if (umip_inst == UMIP_INST_SMSW || umip_inst == UMIP_INST_SLDT > > >|| > > >+ umip_inst == UMIP_INST_STR) { > > >+ unsigned long dummy_value; > > >+ > > >+ if (umip_inst == UMIP_INST_SMSW) { > > >+ dummy_value = CR0_STATE; > > >+ } else if (umip_inst == UMIP_INST_STR) { > > >+ dummy_value = GDT_ENTRY_TSS * 8; > > >+ } else if (umip_inst == UMIP_INST_SLDT) { > > >+#ifdef CONFIG_MODIFY_LDT_SYSCALL > > >+ down_read(¤t->mm->context.ldt_usr_sem); > > >+ if (current->mm->context.ldt) > > >+ dummy_value = GDT_ENTRY_LDT * 8; > > >+ else > > >+ dummy_value = 0; > > >+ up_read(¤t->mm->context.ldt_usr_sem); > > >+#else > > >+ dummy_value = 0; > > >+#endif > > >+ } > > > > > > /* > > >- * Even though the CR0 register has 4 bytes, the number > > >+ * For these 3 instructions, the number > > > * of bytes to be copied in the result buffer is determined > > > * by whether the operand is a register or a memory location. > > > * If operand is a register, return as many bytes as the operand > > > * size. If operand is memory, return only the two least > > >- * siginificant bytes of CR0. > > >+ * siginificant bytes. > > > */ > > > if (X86_MODRM_MOD(insn->modrm.value) == 3) > > > *data_size = insn->opnd_bytes; > > >@@ -261,7 +280,6 @@ static int emulate_umip_insn(struct insn *insn, int > > >umip_inst, > > > *data_size = 2; > > > > > > memcpy(data, &dummy_value, *data_size); > > >- /* STR and SLDT are not emulated */ > > > } else { > > > return -EINVAL; > > > } > > >@@ -383,10 +401,6 @@ bool fixup_umip_exception(struct pt_regs *regs) > > > umip_pr_warn(regs, "%s instruction cannot be used by applications.\n", > > > umip_insns[umip_inst]); > > > > > >- /* Do not emulate (spoof) SLDT or STR. */ > > >- if (umip_inst == UMIP_INST_STR || umip_inst == UMIP_INST_SLDT) > > >- return false; > > >- > > > umip_pr_warn(regs, "For now, expensive software emulation returns the > > >result.\n"); > > > > > > if (emulate_umip_insn(&insn, umip_inst, dummy_data, &dummy_data_size, > > > > It's there any reason for SLDT to not *always* return a fixed value? "An LDT has been assigned" is formally a kernel internal property, separate from the property of whenever there are user space enteies in the LDT. > > But isn't it true that sldt returns 0 if the application has not set an > LDT and non-zero otherwise? > > In native_set_ldt() I see that the the LDT register is set to 0 if the > table has no entries and to GDT_ENTRY_LDT*8 otherwise. > > Please correct me if I understand this wrong.
You're understanding the LDT right, but that's none of the user's business. hpa may well be right -- unless it actively causes problems, we might as well return a constant value.
| |