May 2017 - Wine-Devel - list.winehq.org

[PATCH v7 03/26] x86/mpx: Use signed variables to compute effective addresses

by Ricardo Neri

Even though memory addresses are unsigned, the operands used to compute the effective address do have a sign. This is true for the ModRM.rm, SIB.base, SIB.index as well as the displacement bytes. Thus, signed variables shall be used when computing the effective address from these operands. Once the signed effective address has been computed, it is casted to an unsigned long to determine the linear address. Variables are renamed to better reflect the type of address being computed. Cc: Borislav Petkov <bp(a)suse.de> Cc: Andy Lutomirski <luto(a)kernel.org> Cc: Dave Hansen <dave.hansen(a)linux.intel.com> Cc: Adam Buchbinder <adam.buchbinder(a)gmail.com> Cc: Colin Ian King <colin.king(a)canonical.com> Cc: Lorenzo Stoakes <lstoakes(a)gmail.com> Cc: Qiaowei Ren <qiaowei.ren(a)intel.com> Cc: Peter Zijlstra <peterz(a)infradead.org> Cc: Nathan Howard <liverlint(a)gmail.com> Cc: Adan Hawthorn <adanhawthorn(a)gmail.com> Cc: Joe Perches <joe(a)perches.com> Cc: Ravi V. Shankar <ravi.v.shankar(a)intel.com> Cc: x86(a)kernel.org Signed-off-by: Ricardo Neri <ricardo.neri-calderon(a)linux.intel.com> --- arch/x86/mm/mpx.c | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/arch/x86/mm/mpx.c b/arch/x86/mm/mpx.c index 1c34b76..ebdead8 100644 --- a/arch/x86/mm/mpx.c +++ b/arch/x86/mm/mpx.c @@ -138,7 +138,8 @@ static int get_reg_offset(struct insn *insn, struct pt_regs *regs, */ static void __user *mpx_get_addr_ref(struct insn *insn, struct pt_regs *regs) { - unsigned long addr, base, indx; + unsigned long linear_addr; + long eff_addr, base, indx; int addr_offset, base_offset, indx_offset; insn_byte_t sib; @@ -150,7 +151,7 @@ static void __user *mpx_get_addr_ref(struct insn *insn, struct pt_regs *regs) addr_offset = get_reg_offset(insn, regs, REG_TYPE_RM); if (addr_offset < 0) goto out_err; - addr = regs_get_register(regs, addr_offset); + eff_addr = regs_get_register(regs, addr_offset); } else { if (insn->sib.nbytes) { base_offset = get_reg_offset(insn, regs, REG_TYPE_BASE); @@ -163,16 +164,18 @@ static void __user *mpx_get_addr_ref(struct insn *insn, struct pt_regs *regs) base = regs_get_register(regs, base_offset); indx = regs_get_register(regs, indx_offset); - addr = base + indx * (1 << X86_SIB_SCALE(sib)); + eff_addr = base + indx * (1 << X86_SIB_SCALE(sib)); } else { addr_offset = get_reg_offset(insn, regs, REG_TYPE_RM); if (addr_offset < 0) goto out_err; - addr = regs_get_register(regs, addr_offset); + eff_addr = regs_get_register(regs, addr_offset); } - addr += insn->displacement.value; + eff_addr += insn->displacement.value; } - return (void __user *)addr; + linear_addr = (unsigned long)eff_addr; + + return (void __user *)linear_addr; out_err: return (void __user *)-1; } -- 2.9.3

May 5, 2017

[PATCH v7 02/26] x86/mm: Relocate page fault error codes to traps.h

by Ricardo Neri

Up to this point, only fault.c used the definitions of the page fault error codes. Thus, it made sense to keep them within such file. Other portions of code might be interested in those definitions too. For instance, the User- Mode Instruction Prevention emulation code will use such definitions to emulate a page fault when it is unable to successfully copy the results of the emulated instructions to user space. While relocating the error code enumeration, the prefix X86_ is used to make it consistent with the rest of the definitions in traps.h. Of course, code using the enumeration had to be updated as well. No functional changes were performed. Cc: Thomas Gleixner <tglx(a)linutronix.de> Cc: Ingo Molnar <mingo(a)redhat.com> Cc: "H. Peter Anvin" <hpa(a)zytor.com> Cc: Andy Lutomirski <luto(a)kernel.org> Cc: "Kirill A. Shutemov" <kirill.shutemov(a)linux.intel.com> Cc: Josh Poimboeuf <jpoimboe(a)redhat.com> Cc: Dave Hansen <dave.hansen(a)linux.intel.com> Cc: Paul Gortmaker <paul.gortmaker(a)windriver.com> Cc: x86(a)kernel.org Reviewed-by: Andy Lutomirski <luto(a)kernel.org> Signed-off-by: Ricardo Neri <ricardo.neri-calderon(a)linux.intel.com> --- arch/x86/include/asm/traps.h | 18 +++++++++ arch/x86/mm/fault.c | 88 +++++++++++++++++--------------------------- 2 files changed, 52 insertions(+), 54 deletions(-) diff --git a/arch/x86/include/asm/traps.h b/arch/x86/include/asm/traps.h index 01fd0a7..4a2e585 100644 --- a/arch/x86/include/asm/traps.h +++ b/arch/x86/include/asm/traps.h @@ -148,4 +148,22 @@ enum { X86_TRAP_IRET = 32, /* 32, IRET Exception */ }; +/* + * Page fault error code bits: + * + * bit 0 == 0: no page found 1: protection fault + * bit 1 == 0: read access 1: write access + * bit 2 == 0: kernel-mode access 1: user-mode access + * bit 3 == 1: use of reserved bit detected + * bit 4 == 1: fault was an instruction fetch + * bit 5 == 1: protection keys block access + */ +enum x86_pf_error_code { + X86_PF_PROT = 1 << 0, + X86_PF_WRITE = 1 << 1, + X86_PF_USER = 1 << 2, + X86_PF_RSVD = 1 << 3, + X86_PF_INSTR = 1 << 4, + X86_PF_PK = 1 << 5, +}; #endif /* _ASM_X86_TRAPS_H */ diff --git a/arch/x86/mm/fault.c b/arch/x86/mm/fault.c index 8ad91a0..32f3070 100644 --- a/arch/x86/mm/fault.c +++ b/arch/x86/mm/fault.c @@ -29,26 +29,6 @@ #include <asm/trace/exceptions.h> /* - * Page fault error code bits: - * - * bit 0 == 0: no page found 1: protection fault - * bit 1 == 0: read access 1: write access - * bit 2 == 0: kernel-mode access 1: user-mode access - * bit 3 == 1: use of reserved bit detected - * bit 4 == 1: fault was an instruction fetch - * bit 5 == 1: protection keys block access - */ -enum x86_pf_error_code { - - PF_PROT = 1 << 0, - PF_WRITE = 1 << 1, - PF_USER = 1 << 2, - PF_RSVD = 1 << 3, - PF_INSTR = 1 << 4, - PF_PK = 1 << 5, -}; - -/* * Returns 0 if mmiotrace is disabled, or if the fault is not * handled by mmiotrace: */ @@ -149,7 +129,7 @@ is_prefetch(struct pt_regs *regs, unsigned long error_code, unsigned long addr) * If it was a exec (instruction fetch) fault on NX page, then * do not ignore the fault: */ - if (error_code & PF_INSTR) + if (error_code & X86_PF_INSTR) return 0; instr = (void *)convert_ip_to_linear(current, regs); @@ -179,7 +159,7 @@ is_prefetch(struct pt_regs *regs, unsigned long error_code, unsigned long addr) * siginfo so userspace can discover which protection key was set * on the PTE. * - * If we get here, we know that the hardware signaled a PF_PK + * If we get here, we know that the hardware signaled a X86_PF_PK * fault and that there was a VMA once we got in the fault * handler. It does *not* guarantee that the VMA we find here * was the one that we faulted on. @@ -205,7 +185,7 @@ static void fill_sig_info_pkey(int si_code, siginfo_t *info, /* * force_sig_info_fault() is called from a number of * contexts, some of which have a VMA and some of which - * do not. The PF_PK handing happens after we have a + * do not. The X86_PF_PK handing happens after we have a * valid VMA, so we should never reach this without a * valid VMA. */ @@ -695,7 +675,7 @@ show_fault_oops(struct pt_regs *regs, unsigned long error_code, if (!oops_may_print()) return; - if (error_code & PF_INSTR) { + if (error_code & X86_PF_INSTR) { unsigned int level; pgd_t *pgd; pte_t *pte; @@ -779,7 +759,7 @@ no_context(struct pt_regs *regs, unsigned long error_code, */ if (current->thread.sig_on_uaccess_err && signal) { tsk->thread.trap_nr = X86_TRAP_PF; - tsk->thread.error_code = error_code | PF_USER; + tsk->thread.error_code = error_code | X86_PF_USER; tsk->thread.cr2 = address; /* XXX: hwpoison faults will set the wrong code. */ @@ -899,7 +879,7 @@ __bad_area_nosemaphore(struct pt_regs *regs, unsigned long error_code, struct task_struct *tsk = current; /* User mode accesses just cause a SIGSEGV */ - if (error_code & PF_USER) { + if (error_code & X86_PF_USER) { /* * It's possible to have interrupts off here: */ @@ -920,7 +900,7 @@ __bad_area_nosemaphore(struct pt_regs *regs, unsigned long error_code, * Instruction fetch faults in the vsyscall page might need * emulation. */ - if (unlikely((error_code & PF_INSTR) && + if (unlikely((error_code & X86_PF_INSTR) && ((address & ~0xfff) == VSYSCALL_ADDR))) { if (emulate_vsyscall(regs, address)) return; @@ -933,7 +913,7 @@ __bad_area_nosemaphore(struct pt_regs *regs, unsigned long error_code, * are always protection faults. */ if (address >= TASK_SIZE_MAX) - error_code |= PF_PROT; + error_code |= X86_PF_PROT; if (likely(show_unhandled_signals)) show_signal_msg(regs, error_code, address, tsk); @@ -989,11 +969,11 @@ static inline bool bad_area_access_from_pkeys(unsigned long error_code, if (!boot_cpu_has(X86_FEATURE_OSPKE)) return false; - if (error_code & PF_PK) + if (error_code & X86_PF_PK) return true; /* this checks permission keys on the VMA: */ - if (!arch_vma_access_permitted(vma, (error_code & PF_WRITE), - (error_code & PF_INSTR), foreign)) + if (!arch_vma_access_permitted(vma, (error_code & X86_PF_WRITE), + (error_code & X86_PF_INSTR), foreign)) return true; return false; } @@ -1021,7 +1001,7 @@ do_sigbus(struct pt_regs *regs, unsigned long error_code, unsigned long address, int code = BUS_ADRERR; /* Kernel mode? Handle exceptions or die: */ - if (!(error_code & PF_USER)) { + if (!(error_code & X86_PF_USER)) { no_context(regs, error_code, address, SIGBUS, BUS_ADRERR); return; } @@ -1050,14 +1030,14 @@ mm_fault_error(struct pt_regs *regs, unsigned long error_code, unsigned long address, struct vm_area_struct *vma, unsigned int fault) { - if (fatal_signal_pending(current) && !(error_code & PF_USER)) { + if (fatal_signal_pending(current) && !(error_code & X86_PF_USER)) { no_context(regs, error_code, address, 0, 0); return; } if (fault & VM_FAULT_OOM) { /* Kernel mode? Handle exceptions or die: */ - if (!(error_code & PF_USER)) { + if (!(error_code & X86_PF_USER)) { no_context(regs, error_code, address, SIGSEGV, SEGV_MAPERR); return; @@ -1082,16 +1062,16 @@ mm_fault_error(struct pt_regs *regs, unsigned long error_code, static int spurious_fault_check(unsigned long error_code, pte_t *pte) { - if ((error_code & PF_WRITE) && !pte_write(*pte)) + if ((error_code & X86_PF_WRITE) && !pte_write(*pte)) return 0; - if ((error_code & PF_INSTR) && !pte_exec(*pte)) + if ((error_code & X86_PF_INSTR) && !pte_exec(*pte)) return 0; /* * Note: We do not do lazy flushing on protection key - * changes, so no spurious fault will ever set PF_PK. + * changes, so no spurious fault will ever set X86_PF_PK. */ - if ((error_code & PF_PK)) + if ((error_code & X86_PF_PK)) return 1; return 1; @@ -1137,8 +1117,8 @@ spurious_fault(unsigned long error_code, unsigned long address) * change, so user accesses are not expected to cause spurious * faults. */ - if (error_code != (PF_WRITE | PF_PROT) - && error_code != (PF_INSTR | PF_PROT)) + if (error_code != (X86_PF_WRITE | X86_PF_PROT) && + error_code != (X86_PF_INSTR | X86_PF_PROT)) return 0; pgd = init_mm.pgd + pgd_index(address); @@ -1198,19 +1178,19 @@ access_error(unsigned long error_code, struct vm_area_struct *vma) * always an unconditional error and can never result in * a follow-up action to resolve the fault, like a COW. */ - if (error_code & PF_PK) + if (error_code & X86_PF_PK) return 1; /* * Make sure to check the VMA so that we do not perform - * faults just to hit a PF_PK as soon as we fill in a + * faults just to hit a X86_PF_PK as soon as we fill in a * page. */ - if (!arch_vma_access_permitted(vma, (error_code & PF_WRITE), - (error_code & PF_INSTR), foreign)) + if (!arch_vma_access_permitted(vma, (error_code & X86_PF_WRITE), + (error_code & X86_PF_INSTR), foreign)) return 1; - if (error_code & PF_WRITE) { + if (error_code & X86_PF_WRITE) { /* write, present and write, not present: */ if (unlikely(!(vma->vm_flags & VM_WRITE))) return 1; @@ -1218,7 +1198,7 @@ access_error(unsigned long error_code, struct vm_area_struct *vma) } /* read, present: */ - if (unlikely(error_code & PF_PROT)) + if (unlikely(error_code & X86_PF_PROT)) return 1; /* read, not present: */ @@ -1241,7 +1221,7 @@ static inline bool smap_violation(int error_code, struct pt_regs *regs) if (!static_cpu_has(X86_FEATURE_SMAP)) return false; - if (error_code & PF_USER) + if (error_code & X86_PF_USER) return false; if (!user_mode(regs) && (regs->flags & X86_EFLAGS_AC)) @@ -1297,7 +1277,7 @@ __do_page_fault(struct pt_regs *regs, unsigned long error_code, * protection error (error_code & 9) == 0. */ if (unlikely(fault_in_kernel_space(address))) { - if (!(error_code & (PF_RSVD | PF_USER | PF_PROT))) { + if (!(error_code & (X86_PF_RSVD | X86_PF_USER | X86_PF_PROT))) { if (vmalloc_fault(address) >= 0) return; @@ -1325,7 +1305,7 @@ __do_page_fault(struct pt_regs *regs, unsigned long error_code, if (unlikely(kprobes_fault(regs))) return; - if (unlikely(error_code & PF_RSVD)) + if (unlikely(error_code & X86_PF_RSVD)) pgtable_bad(regs, error_code, address); if (unlikely(smap_violation(error_code, regs))) { @@ -1351,7 +1331,7 @@ __do_page_fault(struct pt_regs *regs, unsigned long error_code, */ if (user_mode(regs)) { local_irq_enable(); - error_code |= PF_USER; + error_code |= X86_PF_USER; flags |= FAULT_FLAG_USER; } else { if (regs->flags & X86_EFLAGS_IF) @@ -1360,9 +1340,9 @@ __do_page_fault(struct pt_regs *regs, unsigned long error_code, perf_sw_event(PERF_COUNT_SW_PAGE_FAULTS, 1, regs, address); - if (error_code & PF_WRITE) + if (error_code & X86_PF_WRITE) flags |= FAULT_FLAG_WRITE; - if (error_code & PF_INSTR) + if (error_code & X86_PF_INSTR) flags |= FAULT_FLAG_INSTRUCTION; /* @@ -1382,7 +1362,7 @@ __do_page_fault(struct pt_regs *regs, unsigned long error_code, * space check, thus avoiding the deadlock: */ if (unlikely(!down_read_trylock(&mm->mmap_sem))) { - if ((error_code & PF_USER) == 0 && + if ((error_code & X86_PF_USER) == 0 && !search_exception_tables(regs->ip)) { bad_area_nosemaphore(regs, error_code, address, NULL); return; @@ -1409,7 +1389,7 @@ __do_page_fault(struct pt_regs *regs, unsigned long error_code, bad_area(regs, error_code, address); return; } - if (error_code & PF_USER) { + if (error_code & X86_PF_USER) { /* * Accessing the stack below %sp is always a bug. * The large cushion allows instructions like enter -- 2.9.3

May 5, 2017

[PATCH v7 01/26] ptrace, x86: Make user_64bit_mode() available to 32-bit builds

by Ricardo Neri

In its current form, user_64bit_mode() can only be used when CONFIG_X86_64 is selected. This implies that code built with CONFIG_X86_64=n cannot use it. If a piece of code needs to be built for both CONFIG_X86_64=y and CONFIG_X86_64=n and wants to use this function, it needs to wrap it in an #ifdef/#endif; potentially, in multiple places. This can be easily avoided with a single #ifdef/#endif pair within user_64bit_mode() itself. Suggested-by: Borislav Petkov <bp(a)suse.de> Cc: Dave Hansen <dave.hansen(a)linux.intel.com> Cc: Adam Buchbinder <adam.buchbinder(a)gmail.com> Cc: Colin Ian King <colin.king(a)canonical.com> Cc: Lorenzo Stoakes <lstoakes(a)gmail.com> Cc: Qiaowei Ren <qiaowei.ren(a)intel.com> Cc: Arnaldo Carvalho de Melo <acme(a)redhat.com> Cc: Masami Hiramatsu <mhiramat(a)kernel.org> Cc: Adrian Hunter <adrian.hunter(a)intel.com> Cc: Kees Cook <keescook(a)chromium.org> Cc: Thomas Garnier <thgarnie(a)google.com> Cc: Peter Zijlstra <peterz(a)infradead.org> Cc: Borislav Petkov <bp(a)suse.de> Cc: Dmitry Vyukov <dvyukov(a)google.com> Cc: Ravi V. Shankar <ravi.v.shankar(a)intel.com> Cc: x86(a)kernel.org Signed-off-by: Ricardo Neri <ricardo.neri-calderon(a)linux.intel.com> --- arch/x86/include/asm/ptrace.h | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/arch/x86/include/asm/ptrace.h b/arch/x86/include/asm/ptrace.h index 2b5d686..ea78a84 100644 --- a/arch/x86/include/asm/ptrace.h +++ b/arch/x86/include/asm/ptrace.h @@ -115,9 +115,9 @@ static inline int v8086_mode(struct pt_regs *regs) #endif } -#ifdef CONFIG_X86_64 static inline bool user_64bit_mode(struct pt_regs *regs) { +#ifdef CONFIG_X86_64 #ifndef CONFIG_PARAVIRT /* * On non-paravirt systems, this is the only long mode CPL 3 @@ -128,8 +128,12 @@ static inline bool user_64bit_mode(struct pt_regs *regs) /* Headers are too twisted for this to go in paravirt.h. */ return regs->cs == __USER_CS || regs->cs == pv_info.extra_user_64bit_cs; #endif +#else /* !CONFIG_X86_64 */ + return false; +#endif } +#ifdef CONFIG_X86_64 #define current_user_stack_pointer() current_pt_regs()->sp #define compat_user_stack_pointer() current_pt_regs()->sp #endif -- 2.9.3

May 5, 2017

[PATCH v7 00/26] x86: Enable User-Mode Instruction Prevention

by Ricardo Neri

This is v7 of this series. The six previous submissions can be found here [1], here [2], here[3], here[4], here[5] and here[6]. This version addresses the comments received in v6 plus improvements of the handling of exceptions unrelated to UMIP as well as corner cases in virtual-8086 mode. Please see details in the change log. === What is UMIP? User-Mode Instruction Prevention (UMIP) is a security feature present in new Intel Processors. If enabled, it prevents the execution of certain instructions if the Current Privilege Level (CPL) is greater than 0. If these instructions were executed while in CPL > 0, user space applications could have access to system-wide settings such as the global and local descriptor tables, the segment selectors to the current task state and the local descriptor table. Hiding these system resources reduces the tools available to craft privilege escalation attacks such as [7]. These are the instructions covered by UMIP: * SGDT - Store Global Descriptor Table * SIDT - Store Interrupt Descriptor Table * SLDT - Store Local Descriptor Table * SMSW - Store Machine Status Word * STR - Store Task Register If any of these instructions is executed with CPL > 0, a general protection exception is issued when UMIP is enabled. === How does it impact applications? We want to have UMIP enabled by default. However, UMIP will change the behavior that certain applications expect from the operating system. For instance, programs running on WineHQ and DOSEMU2 rely on some of these instructions to function. Stas Sergeev found that Microsoft Windows 3.1 and dos4gw use the instruction SMSW when running in virtual-8086 mode[8]. SGDT and SIDT can also run on virtual-8086 mode. In order to not change the behavior of the system. This patchset emulates the SGDT, SIDT and SMSW. This should be sufficient to not break the applications mentioned above. Regarding the two remaining instructions, STR and SLDT, the WineHQ team has shown interest catching the general protection fault and use it as a vehicle to fix broken applications[9]. Furthermore, STR and SLDT can only run in protected and long modes. DOSEMU2 emulates virtual-8086 mode via KVM. No applications will be broken unless DOSEMU2 decides to enable the CR4.UMIP bit in platforms that support it. Also, this should not pose a security risk as no system resouces would be revealed. Instead, code running inside the KVM would only see the KVM's GDT, IDT and MSW. Please note that UMIP is always enabled for both 64-bit and 32-bit Linux builds. However, emulation of the UMIP-protected instructions is not done for 64-bit processes. 64-bit user space applications will receive the SIGSEGV signal when UMIP instructions causes a general protection fault. === How are UMIP-protected instructions emulated? This version keeps UMIP enabled at all times and by default. If a general protection fault caused by the instructions protected by UMIP is detected, such fault will be fixed-up by returning dummy values as follows: * SGDT and SIDT return hard-coded dummy values as the base of the global descriptor and interrupt descriptor tables. These hard-coded values correspond to memory addresses that are near the end of the kernel memory map. This is also the case for virtual-8086 mode tasks. In all my experiments in x86_32, the base of GDT and IDT was always a 4-byte address, even for 16-bit operands. Thus, my emulation code does the same. In all cases, the limit of the table is set to 0. * SMSW returns the value with which the CR0 register is programmed in head_32/64.S at boot time. This is, the following bits are enabled: CR0.0 for Protection Enable, CR.1 for Monitor Coprocessor, CR.4 for Extension Type, which will always be 1 in recent processors with UMIP; CR.5 for Numeric Error, CR0.16 for Write Protect, CR0.18 for Alignment Mask. As per the Intel 64 and IA-32 Architectures Software Developer's Manual, SMSW returns a 16-bit results for memory operands. However, when the operand is a register, the results can be up to CR0[63:0]. Since the emulation code only kicks-in in x86_32, we return up to CR[31:0]. * The proposed emulation code is handles faults that happens in both protected and virtual-8086 mode. * Again, STR and SLDT are not emulated. === How is this series laid out? ++ Preparatory work As per suggestions from Andy Lutormirsky and Borislav Petkov, I moved the x86 page fault error codes to a header. Also, I made user_64bit_mode available to x86_32 builds. This helps to reuse code and reduce the number of #ifdef's in these patches. ++ Fix bugs in MPX address evaluator I found very useful the code for Intel MPX (Memory Protection Extensions) used to parse opcodes and the memory locations contained in the general purpose registers when used as operands. I put some of this code in a separate library file that both MPX and UMIP can access and avoid code duplication. Before creating the new library, I fixed a couple of bugs that I found in how MPX determines the address contained in the instruction and operands. ++ Provide a new x86 instruction evaluating library With bugs fixed, the MPX evaluating code is relocated in a new insn-eval.c library. The basic functionality of this library is extended to obtain the segment descriptor selected by either segment override prefixes or the default segment by the involved registers in the calculation of the effective address. It was also extended to obtain the default address and operand sizes as well as the segment base address. Also, support to process 16-bit address encodings. Armed with this arsenal, it is now possible to determine the linear address onto which the emulated results shall be copied. This code supports Normal 32-bit and 64-bit (i.e., __USER32_CS and/or __USER_CS) protected mode, virtual-8086 mode, 16-bit protected mode with 32-bit base address. ++ Emulate UMIP instructions A new fixup_umip_exception functions inspect the instruction at the instruction pointer. If it is an UMIP-protected instruction, it executes the emulation code. This uses all the address-computing code of the previous section. ++ Add self-tests Lastly, self-tests are added to entry_from_v86.c to exercise the most typical use cases of UMIP-protected instructions in a virtual-8086 mode. ++ Extensive tests Extensive tests were performed to test all the combinations of ModRM, SiB and displacements for 16-bit and 32-bit encodings for the ss, ds, es, fs and gs segments. Tests also include a 64-bit program that uses segmentation via fs and gs. For this purpose, I temporarily enabled UMIP support for 64-bit process. This change is not part of this patchset. The intention is to test the computations of linear addresses in 64-bit mode, including the extra R8-R15 registers. Extensive test is also implemented for virtual-8086 tasks. Code of these tests can be found here [10] and here [11]. ++ Merging this series? Am I any close to see these patches merged? :) [1]. https://lwn.net/Articles/705877/ [2]. https://lkml.org/lkml/2016/12/23/265 [3]. https://lkml.org/lkml/2017/1/25/622 [4]. https://lkml.org/lkml/2017/2/23/40 [5]. https://lkml.org/lkml/2017/3/3/678 [6]. https://lkml.org/lkml/2017/3/7/866 [7]. http://timetobleed.com/a-closer-look-at-a-recent-privilege-escalation-bug-i… [8]. https://www.winehq.org/pipermail/wine-devel/2017-April/117159.html [10]. https://github.com/01org/luv-yocto/tree/rneri/umip/meta-luv/recipes-core/um… [11]. https://github.com/01org/luv-yocto/commit/a72a7fe7d68693c0f4100ad86de6ecabd… Thanks and BR, Ricardo Changes since V6: *Reworded and addded more details on the special cases of ModRM and SIB bytes. To avoid confusion, I ommited mentioning the involved registers (EBP and ESP). *Replaced BUG() with printk_ratelimited in function get_reg_offset of insn-eval.c *Removed unused utility functions that obtain a register value from pt_regs given a SIB base and index. *Clarified nomenclature to call CS, DS, ES, FS, GS and SS segment registers and their values segment selectors. *Reworked function resolve_seg_register to issue an error when more than one segment overrides prefixes are used in the instruction. *Added logic in resolve_seg_register to ignore segment register when in long mode and not using FS or GS. *Added logic to ensure the effective address is within the limits of the segment in protected mode. *Added logic to ensure segment override prefixes are ignored when resolving the segment of EIP and EDI with string instructions. *Added code to make user_64bit_mode() available in CONFIG_X86_32... and make it return false, of course. *Merged the two functions that obtain the default address and operand size of a code segment into one as they are always used together. *Corrected logic of displacement-only addressing in long mode to make the displacement relative to the RIP of the next instruction. *Reworked logic to sign-extend 32-bit memory offsets into 64-bit signed memory offsets. This include more checks and putting all together in an utility function. *Removed the 'unlikely' of conditional statements as we are not in a critical path. *In virtual-8086 mode, ensure that effective addresses are always less than 0x10000, even when address override prefixes are used. Also, ensure that linear addresses have a size of 20-bits. Changes since V5: * Relocate the page fault error code enumerations to traps.h Changes since V4: * Audited patches to use braces in all the branches of conditional. statements, except those in which the conditional action only takes one line. * Implemented support in 64-builds for both 32-bit and 64-bit tasks in the instruction evaluating library. * Split segment selector function in the instruction evaluating library into two functions to resolve the segment type by instruction override or default and a separate function to actually read the segment selector. * Fixed a bug when evaluating 32-bit effective addresses with 64-bit kernels. * Split patches further for for easier review. * Use signed variables for computation of effective address. * Fixed issue with a spurious static modifier in function insn_get_addr_ref found by kbuild test bot. * Removed comparison between true and fixup_umip_exception. * Reworked check logic when identifying erroneous vs invalid values of the SiB base and index. Changes since V3: * Limited emulation to 32-bit and 16-bit modes. For 64-bit mode, a general protection fault is still issued when UMIP-protected instructions are executed with CPL > 0. * Expanded instruction-evaluating code to obtain segment descriptor along with their attributes such as base address and default address and operand sizes. Also, support for 16-bit encodings in protected mode was implemented. * When getting a segment descriptor, this include support to obtain those of a local descriptor table. * Now the instruction-evaluating code returns -EDOM when the value of registers should not be used in calculating the effective address. The value -EINVAL is left for errors. * Incorporate the value of the segment base address in the computation of linear addresses. * Renamed new instruction evaluation library from insn-kernel.c to insn-eval.c * Exported functions insn_get_reg_offset_* to obtain the register offset by ModRM r/m, SiB base and SiB index. * Improved documentation of functions. * Split patches further for easier review. Changes since V2: * Added new utility functions to decode the memory addresses contained in registers when the 16-bit addressing encodings are used. This includes code to obtain and compute memory addresses using segment selectors for real-mode address translation. * Added support to emulate UMIP-protected instructions for virtual-8086 tasks. * Added self-tests for virtual-8086 mode that contains representative use cases: address represented as a displacement, address in registers and registers as operands. * Instead of maintaining a static variable for the dummy base addresses of the IDT and GDT, a hard-coded value is used. * The emulated SMSW instructions now return the value with which the CR0 register is programmed in head_32/64.S This is: PE | MP | ET | NE | WP | AM. For x86_64, PG is also enabled. * The new file arch/x86/lib/insn-utils.c is now renamed as arch/x86/lib/ insn-kernel.c. It also has its own header. This helps keep in sync the the kernel and objtool instruction decoders. Also, the new insn-kernel.c contains utility functions that are only relevant in a kernel context. * Removed printed warnings for errors that occur when decoding instructions with invalid operands. * Added more comments on fixes in the instruction-decoding MPX functions. * Now user_64bit_mode(regs) is used instead of test_thread_flag(TIF_IA32) to determine if the task is 32-bit or 64-bit. * Found and fixed a bug in insn-decoder in which X86_MODRM_RM was incorrectly used to obtain the mod part of the ModRM byte. * Added more explanatory code in emulation and instruction decoding code. This includes a comment regarding that copy_from_user could fail if there exists a memory protection key in place. * Tested code with CONFIG_X86_DECODER_SELFTEST=y and everything passes now. * Prefixed get_reg_offset_rm with insn_ as this function is exposed via a header file. For clarity, this function was added in a separate patch. Changes since V1: * Virtual-8086 mode tasks are not treated in a special manner. All code for this purpose was removed. * Instead of attempting to disable UMIP during a context switch or when entering virtual-8086 mode, UMIP remains enabled all the time. General protection faults that occur are fixed-up by returning dummy values as detailed above. * Removed umip= kernel parameter in favor of using clearcpuid=514 to disable UMIP. * Removed selftests designed to detect the absence of SIGSEGV signals when running in virtual-8086 mode. * Reused code from MPX to decode instructions operands. For this purpose code was put in a common location. * Fixed two bugs in MPX code that decodes operands. Ricardo Neri (26): ptrace,x86: Make user_64bit_mode() available to 32-bit builds x86/mm: Relocate page fault error codes to traps.h x86/mpx: Use signed variables to compute effective addresses x86/mpx: Do not use SIB.index if its value is 100b and ModRM.mod is not 11b x86/mpx: Do not use SIB.base if its value is 101b and ModRM.mod = 0 x86/mpx, x86/insn: Relocate insn util functions to a new insn-eval file x86/insn-eval: Do not BUG on invalid register type x86/insn-eval: Add a utility function to get register offsets x86/insn-eval: Add utility function to identify string instructions x86/insn-eval: Add utility functions to get segment selector x86/insn-eval: Add utility function to get segment descriptor x86/insn-eval: Add utility functions to get segment descriptor base address and limit x86/insn-eval: Add function to get default params of code segment x86/insn-eval: Indicate a 32-bit displacement if ModRM.mod is 0 and ModRM.rm is 5 x86/insn-eval: Incorporate segment base and limit in linear address computation x86/insn-eval: Support both signed 32-bit and 64-bit effective addresses x86/insn-eval: Handle 32-bit address encodings in virtual-8086 mode x86/insn-eval: Add support to resolve 16-bit addressing encodings x86/insn-eval: Add wrapper function for 16-bit and 32-bit address encodings x86/cpufeature: Add User-Mode Instruction Prevention definitions x86: Add emulation code for UMIP instructions x86/umip: Force a page fault when unable to copy emulated result to user x86/traps: Fixup general protection faults caused by UMIP x86: Enable User-Mode Instruction Prevention selftests/x86: Add tests for User-Mode Instruction Prevention selftests/x86: Add tests for instruction str and sldt arch/x86/Kconfig | 10 + arch/x86/include/asm/cpufeatures.h | 1 + arch/x86/include/asm/disabled-features.h | 8 +- arch/x86/include/asm/insn-eval.h | 25 + arch/x86/include/asm/ptrace.h | 6 +- arch/x86/include/asm/traps.h | 18 + arch/x86/include/asm/umip.h | 15 + arch/x86/include/uapi/asm/processor-flags.h | 2 + arch/x86/kernel/Makefile | 1 + arch/x86/kernel/cpu/common.c | 16 +- arch/x86/kernel/traps.c | 4 + arch/x86/kernel/umip.c | 286 +++++++ arch/x86/lib/Makefile | 2 +- arch/x86/lib/insn-eval.c | 1066 +++++++++++++++++++++++++ arch/x86/mm/fault.c | 88 +- arch/x86/mm/mpx.c | 120 +-- tools/testing/selftests/x86/entry_from_vm86.c | 89 ++- 17 files changed, 1580 insertions(+), 177 deletions(-) create mode 100644 arch/x86/include/asm/insn-eval.h create mode 100644 arch/x86/include/asm/umip.h create mode 100644 arch/x86/kernel/umip.c create mode 100644 arch/x86/lib/insn-eval.c -- 2.9.3

May 5, 2017

Re: [v6 PATCH 08/21] x86/insn-eval: Add utility function to get segment descriptor base address

by Borislav Petkov

On Wed, Apr 26, 2017 at 03:52:41PM -0700, Ricardo Neri wrote: > Probably insn_get_seg_base() itself can verify if there are segment > override prefixes in the struct insn. If yes, use them except for > specific cases such as CS. ... and depending on whether in long mode or not. > On an unrelated note, I still have the problem of using DS vs ES for > string instructions. Perhaps instead of a use_default_seg flag, a > string_instruction flag that indicates how to determine the default > segment. ... or you can look at the insn opcode directly. AFAICT, you need to check whether the opcode is 0xa4 or 0xa5 and that the insn is a single-byte opcode, i.e., not from the secondary map escaped with 0xf or some of the other multi-byte opcode maps. -- Regards/Gruss, Boris. SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg) --

May 5, 2017

Re: [v6 PATCH 08/21] x86/insn-eval: Add utility function to get segment descriptor base address

by Borislav Petkov

On Wed, Apr 26, 2017 at 03:37:44PM -0700, Ricardo Neri wrote: > I need a human-readable way of identifying what segment selector (in > pt_regs, vm86regs or directly reading the segment registers) to use. > Since there is a segment override prefix for all of them, I thought I > could use them. Yes, you should... > Perhaps I can rename enum segment to enum segment_selector and comment > that the values in the enum are those of the override prefixes. Would > that be reasonable? ... but you should call them what they are: "enum seg_override_pfxs" or "enum seg_ovr_pfx" or... Or somesuch. I suck at naming stuff. -- Regards/Gruss, Boris. SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg) --

May 5, 2017

Re: [PATCH] msvcp90: Call invalid_parameter_handler on index out of range in basic_string::operator[]

by Piotr Caban

On 05/05/17 19:01, Piotr Caban wrote: > Signed-off-by: Piotr Caban <piotr(a)codeweavers.com> > --- > dlls/msvcp90/string.c | 13 ++++++++----- > 1 file changed, 8 insertions(+), 5 deletions(-) Please ignore this patch for now. Thanks, Piotr

May 5, 2017

Re: [PATCH] wined3d: Add NVIDIA GTX 1080 Ti to the GPU list

by Henri Verbeet

On 4 May 2017 at 22:53, Björn Bidar <theodorstormgrade(a)googlemail.com> wrote: DRIVER_NVIDIA_GEFORCE8, 8192}, > + {HW_VENDOR_NVIDIA, CARD_NVIDIA_GEFORCE_GTX1080TI, "NVIDIA GeForce GTX 1080 Ti", DRIVER_NVIDIA_GEFORCE8, 11169}, That's an unusual amount of memory, should that be 11264?

May 5, 2017

Re: [PATCH] user32: Removed 16-bit windows position trunc in WINPOS_SetPlacement.

by Alexandre Julliard

Kees Beets <kbeets68(a)gmail.com> writes: > Fixes https://bugs.winehq.org/show_bug.cgi?id=38231 > > Replaced SetWindowPos in WINPOS_SetPlacement with an implementation that > does not truncate windows positions to 16-bit. Fixes a bug in Adobe > Lightroom under wine that is not present under windows (7, 8, 10), > while still running all win tests successfully. It's good that it doesn't break the other tests, but since we currently don't have tests for SetWindowPlacement, that's not very convincing. You'd want to add some tests at least for the truncation thing, and also probably some for the message sequences. -- Alexandre Julliard julliard(a)winehq.org

May 5, 2017

Re: [PATCH 04/10] d3dx9/tests: Use compare_float() in test_D3DXSHEvalSphericalLight().

by Alexandre Julliard

Henri Verbeet <hverbeet(a)codeweavers.com> writes: > Signed-off-by: Henri Verbeet <hverbeet(a)codeweavers.com> > --- > dlls/d3dx9_36/tests/math.c | 171 ++++++++++++++++++++++++--------------------- > 1 file changed, 92 insertions(+), 79 deletions(-) This one is failing here: ../../../tools/runtest -q -P wine -T ../../.. -M d3dx9_36.dll -p d3dx9_36_test.exe.so math && touch math.ok math.c:3194: Test failed: Red: case 1, order 5: expected[20] = 5.29080920e-04, received 5.29033889e-04. math.c:3205: Test failed: Green: case 1, order 5: expected[20] = 8.09182529e-04, received 8.09110585e-04. math.c:3217: Test failed: Blue: case 1, order 5: expected[20] = 1.08928420e-03, received 1.08918734e-03. math.c:3194: Test failed: Red: case 1, order 6: expected[20] = 5.29080920e-04, received 5.29033889e-04. math.c:3205: Test failed: Green: case 1, order 6: expected[20] = 8.09182529e-04, received 8.09110585e-04. math.c:3217: Test failed: Blue: case 1, order 6: expected[20] = 1.08928420e-03, received 1.08918734e-03. math.c:3194: Test failed: Red: case 2, order 5: expected[20] = -7.14543217e-04, received -7.14479480e-04. math.c:3217: Test failed: Blue: case 2, order 5: expected[20] = -1.47111830e-03, received -1.47098722e-03. math.c:3194: Test failed: Red: case 2, order 6: expected[20] = -7.14543217e-04, received -7.14479480e-04. math.c:3217: Test failed: Blue: case 2, order 6: expected[20] = -1.47111830e-03, received -1.47098722e-03. Makefile:340: recipe for target 'math.ok' failed make[1]: *** [math.ok] Error 10 -- Alexandre Julliard julliard(a)winehq.org

May 4, 2017