Messages in this thread Patch in this message | | | Date | Fri, 5 Jun 2020 15:42:31 -0700 | From | Ricardo Neri <> | Subject | Re: [PATCH] x86/umip: Add emulation/spoofing for SLDT and STR instructions |
| |
On Fri, Jun 05, 2020 at 11:58:13AM -0700, Brendan Shanks wrote: > > > On Jun 3, 2020, at 9:39 PM, Andy Lutomirski <luto@amacapital.net> wrote: > > > > On Wed, Jun 3, 2020 at 5:12 PM Ricardo Neri > > <ricardo.neri-calderon@linux.intel.com <mailto:ricardo.neri-calderon@linux.intel.com>> wrote: > >> > >> On Tue, Jun 02, 2020 at 11:42:12AM -0700, Brendan Shanks 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> > >>> --- > >>> arch/x86/kernel/umip.c | 23 ++++++++++++++--------- > >>> 1 file changed, 14 insertions(+), 9 deletions(-) > >>> > >>> diff --git a/arch/x86/kernel/umip.c b/arch/x86/kernel/umip.c > >>> index 8d5cbe1bbb3b..59dfceac5cc0 100644 > >>> --- a/arch/x86/kernel/umip.c > >>> +++ b/arch/x86/kernel/umip.c > >>> @@ -64,6 +64,8 @@ > >>> #define UMIP_DUMMY_GDT_BASE 0xfffffffffffe0000ULL > >>> #define UMIP_DUMMY_IDT_BASE 0xffffffffffff0000ULL > >>> > >>> +#define UMIP_DUMMY_TASK_REGISTER_SELECTOR 0x40 > >>> + > >>> /* > >>> * The SGDT and SIDT instructions store the contents of the global descriptor > >>> * table and interrupt table registers, respectively. The destination is a > >>> @@ -244,16 +246,24 @@ 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 = UMIP_DUMMY_TASK_REGISTER_SELECTOR; > >>> + else > >>> + dummy_value = 0; > >> > >> Perhaps you can return a non-zero value for SLDT if it has an LDT, as > >> Andy had suggested. Maybe this can be implemented by looking at > >> current->mm->context.ldt > >> > >> I guess the non-zero value can be (GDT_ENTRY_LDT*8). > > > > You could probably even get away with always returning a nonzero > > value. After all, an empty LDT is quite similar to no LDT. > > > Is something like this what you both had in mind?
> I don’t have any software handy to test the LDT-present case though.
Perhaps you can insert a test in the kernel selftest. Something like this (based on Andreas' test program):
--- a/tools/testing/selftests/x86/ldt_gdt.c +++ b/tools/testing/selftests/x86/ldt_gdt.c @@ -220,12 +220,23 @@ static void install_invalid(const struct user_desc *desc, bool oldmode) } }
+unsigned long test(void) +{ + unsigned char ldtr[5] = "\xef\xbe\xad\xde"; + unsigned long ldt = 0; + asm("sldt %0\n" : "=m" (ldtr)); + ldt = *((unsigned long *)&ldtr[0]); + printf ("LDT base: 0x%lx\n", ldt); + return (ldt); +} + static int safe_modify_ldt(int func, struct user_desc *ptr, unsigned long bytecount) { int ret = syscall(SYS_modify_ldt, 0x11, ptr, bytecount); if (ret < -1) errno = -ret; + test(); return ret; }
Thanks and BR, Ricardo > > else if (umip_inst == UMIP_INST_STR) > dummy_value = UMIP_DUMMY_TASK_REGISTER_SELECTOR; > 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 >
It looks fine to me. Perhaps Andy prefers a simpler, always-non-zero implementation?
Thanks and BR, Ricardo
| |