Re: [v6 PATCH 00/21] x86: Enable User-Mode Instruction Prevention
On Fri, Mar 31, 2017 at 2:26 PM, Stas Sergeev wrote: > 31.03.2017 17:11, Alexandre Julliard пишет: >> >> In fact it would be nice to be able to make sidt/sgdt/etc. segfault >> too. I know a new syscall is a pain, > > Maybe arch_prctl() then? I still like my idea of a generic mechanism to turn off backwards-compatibility things. After all, hardened programs should turn off UMIP fixups entirely. They should also turn off vsyscall emulation entirely, and I see no reason that these mechanisms should be different. --Andy -- To unsubscribe from this list: send the line "unsubscribe linux-msdos" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [v6 PATCH 00/21] x86: Enable User-Mode Instruction Prevention
On Fri, Mar 10, 2017 at 2:30 AM, Stas Sergeev wrote: > 10.03.2017 05:41, Andy Lutomirski пишет: > >> On Wed, Mar 8, 2017 at 5:11 PM, Ricardo Neri >> wrote: >>> >>> On Wed, 2017-03-08 at 19:53 +0300, Stas Sergeev wrote: >>>> >>>> 08.03.2017 19:46, Andy Lutomirski пишет: >>>>>> >>>>>> No no, since I meant prot mode, this is not what I need. >>>>>> I would never need to disable UMIP as to allow the >>>>>> prot mode apps to do SLDT. Instead it would be good >>>>>> to have an ability to provide a replacement for the dummy >>>>>> emulation that is currently being proposed for kernel. >>>>>> All is needed for this, is just to deliver a SIGSEGV. >>>>> >>>>> That's what I meant. Turning off FIXUP_UMIP would leave UMIP on but >>>>> turn off the fixup, so you'd get a SIGSEGV indicating #GP (or a vm86 >>>>> GP exit). >>>> >>>> But then I am confused with the word "compat" in >>>> your "COMPAT_MASK0_X86_UMIP_FIXUP" and >>>> "sys_adjust_compat_mask(int op, int word, u32 mask);" >>>> >>>> Leaving UMIP on and only disabling a fixup doesn't >>>> sound like a compat option to me. I would expect >>>> compat to disable it completely. >>> >>> I guess that the _UMIP_FIXUP part makes it clear that emulation, not >>> UMIP is disabled, allowing the SIGSEGV be delivered to the user space >>> program. >>> >>> Would having a COMPAT_MASK0_X86_UMIP_FIXUP to disable emulation and a >>> COMPAT_MASK0_X86_UMIP to disable UMIP make sense? >>> >>> Also, wouldn't having a COMPAT_MASK0_X86_UMIP to disable UMIP defeat its >>> purpose? Applications could simply use this compat mask to bypass UMIP >>> and gain access to the instructions it protects. >>> >> I was obviously extremely unclear. The point of the proposed syscall >> is to let programs opt out of legacy features. > > I guess both "compat" and "legacy" are misleading > here. Maybe these are "x86-specific" or "hypervisor-specific", > but a mere enabling of UMIP doesn't immediately make > the use of SLDT instruction a legacy IMHO. Sure it is. :) Using SLDT from user mode is a legacy ability that just happens to still work on existing CPUs and kernels. Once UMIP goes in, it will officially be obsolete -- it will just be supported for backwards compatibility. New code should opt out and emulate in usermode if needed. (And the vast, vast majority of Linux programs don't use these instructions in the first place.) Similarly, vsyscalls were obsolete the as soon as better alternatives were fully supported and the kernel started making them slow, and the fact that new static glibc programs still used them for a little while didn't make them any less obsolete. > >> I'll ponder this a bit more. > > So if we are to invent something new, it would be nice to > also think up a clear terminology for it. Maybe something > like "X86_FEATURE_xxx_MASK" or alike. But they're misfeatures, not features. --Andy -- Andy Lutomirski AMA Capital Management, LLC -- To unsubscribe from this list: send the line "unsubscribe linux-msdos" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [v6 PATCH 00/21] x86: Enable User-Mode Instruction Prevention
On Fri, Mar 10, 2017 at 3:33 AM, Stas Sergeev wrote: > 10.03.2017 05:39, Andy Lutomirski пишет: > >> On Thu, Mar 9, 2017 at 2:10 PM, Stas Sergeev wrote: >>> >>> 09.03.2017 04:15, Ricardo Neri пишет: >>> >>>> On Wed, 2017-03-08 at 08:46 -0800, Andy Lutomirski wrote: >>>>> >>>>> On Wed, Mar 8, 2017 at 8:29 AM, Stas Sergeev wrote: >>>>>> >>>>>> 08.03.2017 19:06, Andy Lutomirski пишет: >>>>>>> >>>>>>> On Wed, Mar 8, 2017 at 6:08 AM, Stas Sergeev wrote: >>>>>>>> >>>>>>>> 08.03.2017 03:32, Ricardo Neri пишет: >>>>>>>>> >>>>>>>>> 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 >>>>>>>>> >>>>>>>>> This patchset initially treated tasks running in virtual-8086 >>>>> >>>>> mode as a >>>>>>>>> >>>>>>>>> special case. However, I received clarification that DOSEMU[8] >>>>> >>>>> does not >>>>>>>>> >>>>>>>>> support applications that use these instructions. >>>>>>> >>>>>>> Can you remind me what was special about it? It looks like you >>>>> >>>>> still >>>>>>> >>>>>>> emulate them in v8086 mode. >>>>>> >>>>>> Indeed, sorry, I meant prot mode here. :) >>>>>> So I wonder what was cited to be special about v86. >>>> >>>> Initially my patches disabled UMIP on virtual-8086 instructions, without >>>> regards of protected mode (i.e., UMIP was always enabled). I didn't have >>>> emulation at the time. Then, I added emulation code that now covers >>>> protected and virtual-8086 modes. I guess it is not special anymore. >>> >>> But isn't SLDT&friends just throw UD in v86? >>> How does UMIP affect this? How does your patch affect >>> this? >> >> Er, right. Ricardo, your code may need fixing. But don't you have a >> test case for this? > > Why would you need one? > Or do you really want to allow these instructions > in v86 by the means of emulation? If so - this wasn't > clearly stated in the patch description, neither it was > properly discussed, it seems. What I meant was: if the patches incorrectly started making these instructions work in vm86 mode where they used to cause a vm86 exit, then that's a bug that the selftest should have caught. -- To unsubscribe from this list: send the line "unsubscribe linux-msdos" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [v6 PATCH 00/21] x86: Enable User-Mode Instruction Prevention
On Wed, Mar 8, 2017 at 5:11 PM, Ricardo Neri wrote: > On Wed, 2017-03-08 at 19:53 +0300, Stas Sergeev wrote: >> 08.03.2017 19:46, Andy Lutomirski пишет: >> >> No no, since I meant prot mode, this is not what I need. >> >> I would never need to disable UMIP as to allow the >> >> prot mode apps to do SLDT. Instead it would be good >> >> to have an ability to provide a replacement for the dummy >> >> emulation that is currently being proposed for kernel. >> >> All is needed for this, is just to deliver a SIGSEGV. >> > That's what I meant. Turning off FIXUP_UMIP would leave UMIP on but >> > turn off the fixup, so you'd get a SIGSEGV indicating #GP (or a vm86 >> > GP exit). >> But then I am confused with the word "compat" in >> your "COMPAT_MASK0_X86_UMIP_FIXUP" and >> "sys_adjust_compat_mask(int op, int word, u32 mask);" >> >> Leaving UMIP on and only disabling a fixup doesn't >> sound like a compat option to me. I would expect >> compat to disable it completely. > > I guess that the _UMIP_FIXUP part makes it clear that emulation, not > UMIP is disabled, allowing the SIGSEGV be delivered to the user space > program. > > Would having a COMPAT_MASK0_X86_UMIP_FIXUP to disable emulation and a > COMPAT_MASK0_X86_UMIP to disable UMIP make sense? > > Also, wouldn't having a COMPAT_MASK0_X86_UMIP to disable UMIP defeat its > purpose? Applications could simply use this compat mask to bypass UMIP > and gain access to the instructions it protects. > I was obviously extremely unclear. The point of the proposed syscall is to let programs opt out of legacy features. So there would be a bit to disable emulation of UMIP-blocked instructions (this giving the unadulterated #GP). There would not be a bit to disable UMIP itself. There's also a flaw in my proposal. Disable-vsyscall would be per-mm and disable-umip-emulation would be per-task, so they'd need to be in separate words to make any sense. I'll ponder this a bit more. -- To unsubscribe from this list: send the line "unsubscribe linux-msdos" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [v6 PATCH 00/21] x86: Enable User-Mode Instruction Prevention
On Thu, Mar 9, 2017 at 2:10 PM, Stas Sergeev wrote: > 09.03.2017 04:15, Ricardo Neri пишет: > >> On Wed, 2017-03-08 at 08:46 -0800, Andy Lutomirski wrote: >>> >>> On Wed, Mar 8, 2017 at 8:29 AM, Stas Sergeev wrote: >>>> >>>> 08.03.2017 19:06, Andy Lutomirski пишет: >>>>> >>>>> On Wed, Mar 8, 2017 at 6:08 AM, Stas Sergeev wrote: >>>>>> >>>>>> 08.03.2017 03:32, Ricardo Neri пишет: >>>>>>> >>>>>>> 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 >>>>>>> >>>>>>> This patchset initially treated tasks running in virtual-8086 >>> >>> mode as a >>>>>>> >>>>>>> special case. However, I received clarification that DOSEMU[8] >>> >>> does not >>>>>>> >>>>>>> support applications that use these instructions. >>>>> >>>>> Can you remind me what was special about it? It looks like you >>> >>> still >>>>> >>>>> emulate them in v8086 mode. >>>> >>>> Indeed, sorry, I meant prot mode here. :) >>>> So I wonder what was cited to be special about v86. >> >> Initially my patches disabled UMIP on virtual-8086 instructions, without >> regards of protected mode (i.e., UMIP was always enabled). I didn't have >> emulation at the time. Then, I added emulation code that now covers >> protected and virtual-8086 modes. I guess it is not special anymore. > > But isn't SLDT&friends just throw UD in v86? > How does UMIP affect this? How does your patch affect > this? Er, right. Ricardo, your code may need fixing. But don't you have a test case for this? The behavior should be the same with and without your patches applied. The exception is #UD, not #GP, so maybe your code just never executes in the vm86 case. --Andy -- To unsubscribe from this list: send the line "unsubscribe linux-msdos" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [v6 PATCH 00/21] x86: Enable User-Mode Instruction Prevention
On Wed, Mar 8, 2017 at 8:29 AM, Stas Sergeev wrote: > 08.03.2017 19:06, Andy Lutomirski пишет: >> >> On Wed, Mar 8, 2017 at 6:08 AM, Stas Sergeev wrote: >>> >>> 08.03.2017 03:32, Ricardo Neri пишет: >>>> >>>> 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 >>>> >>>> This patchset initially treated tasks running in virtual-8086 mode as a >>>> special case. However, I received clarification that DOSEMU[8] does not >>>> support applications that use these instructions. >> >> Can you remind me what was special about it? It looks like you still >> emulate them in v8086 mode. > > Indeed, sorry, I meant prot mode here. :) > So I wonder what was cited to be special about v86. Not sure. Ricardo? > >>> Yes, this is the case. >>> But at least in the past there was an attempt to >>> support SLDT as it is used by an ancient pharlap >>> DOS extender (currently unsupported by dosemu1/2). >>> So how difficult would it be to add an optional >>> possibility of delivering such SIGSEGV to userspace >>> so that the kernel's dummy emulation can be overridden? >>> It doesn't need to be a matter of this particular >>> patch set, i.e. this proposal should not trigger a >>> v7 resend of all 21 patches. :) But it would be useful >>> for the future development of dosemu2. >> >> What I'd actually like to see is a totally separate patchset that adds >> an inheritable (but reset on exec) per-task mask of legacy >> compatibility features to disable. Maybe: >> >> sys_adjust_compat_mask(int op, int word, u32 mask); > > No no, since I meant prot mode, this is not what I need. > I would never need to disable UMIP as to allow the > prot mode apps to do SLDT. Instead it would be good > to have an ability to provide a replacement for the dummy > emulation that is currently being proposed for kernel. > All is needed for this, is just to deliver a SIGSEGV. That's what I meant. Turning off FIXUP_UMIP would leave UMIP on but turn off the fixup, so you'd get a SIGSEGV indicating #GP (or a vm86 GP exit). --Andy -- To unsubscribe from this list: send the line "unsubscribe linux-msdos" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [v6 PATCH 15/21] x86/mm: Relocate page fault error codes to traps.h
On Tue, Mar 7, 2017 at 4:32 PM, Ricardo Neri wrote: > 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. > Reviewed-by: Andy Lutomirski -- To unsubscribe from this list: send the line "unsubscribe linux-msdos" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [v6 PATCH 00/21] x86: Enable User-Mode Instruction Prevention
On Wed, Mar 8, 2017 at 6:08 AM, Stas Sergeev wrote: > 08.03.2017 03:32, Ricardo Neri пишет: >> >> 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 >> >> This patchset initially treated tasks running in virtual-8086 mode as a >> special case. However, I received clarification that DOSEMU[8] does not >> support applications that use these instructions. Can you remind me what was special about it? It looks like you still emulate them in v8086 mode. > > Yes, this is the case. > But at least in the past there was an attempt to > support SLDT as it is used by an ancient pharlap > DOS extender (currently unsupported by dosemu1/2). > So how difficult would it be to add an optional > possibility of delivering such SIGSEGV to userspace > so that the kernel's dummy emulation can be overridden? > It doesn't need to be a matter of this particular > patch set, i.e. this proposal should not trigger a > v7 resend of all 21 patches. :) But it would be useful > for the future development of dosemu2. What I'd actually like to see is a totally separate patchset that adds an inheritable (but reset on exec) per-task mask of legacy compatibility features to disable. Maybe: sys_adjust_compat_mask(int op, int word, u32 mask); op could indicate that we want to so SET, OR, AND, or READ. word would be 0 for now. It could be a prctl, too. Things in the mask could include: COMPAT_MASK0_X86_64_VSYSCALL [1] COMPAT_MASK0_X86_UMIP_FIXUP I'm sure I could think of more along these lines. Then DOSEMU (and future WINE versions, too) could just mask off X86_UMIP_FIXUP and do their own emulation [1] For those of you thinking about this and realizing that VSYSCALL readability is inherently global and not per-task, I know how to fix that for essentially no cost :) --Andy -- To unsubscribe from this list: send the line "unsubscribe linux-msdos" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [v6 PATCH 00/21] x86: Enable User-Mode Instruction Prevention
On Tue, Mar 7, 2017 at 4:32 PM, Ricardo Neri wrote: > This is v6 of this series. The five previous submissions can be found > here [1], here [2], here[3], here[4], and here[5]. This version addresses > the comments received in v4 plus improvements of the handling of emulation > in 64-bit builds. Please see details in the change log. > Hi Ingo and Thomas- I think this series is in good enough shape that you should consider making a topic branch (x86/umip?) for it so that it can soak in -next and further development can be done incrementally. In the unlikely event that a major problem shows up, you could skip the pull request to Linus for a cycle. --Andy -- To unsubscribe from this list: send the line "unsubscribe linux-msdos" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [v6 PATCH 19/21] x86/traps: Fixup general protection faults caused by UMIP
On Tue, Mar 7, 2017 at 4:32 PM, Ricardo Neri wrote: > If the User-Mode Instruction Prevention CPU feature is available and > enabled, a general protection fault will be issued if the instructions > sgdt, sldt, sidt, str or smsw are executed from user-mode context > (CPL > 0). If the fault was caused by any of the instructions protected > by UMIP, fixup_umip_exception will emulate dummy results for these > instructions. If emulation is successful, the result is passed to the > user space program and no SIGSEGV signal is emitted. > > Please note that fixup_umip_exception also caters for the case when > the fault originated while running in virtual-8086 mode. Reviewed-by: Andy Lutomirski -- To unsubscribe from this list: send the line "unsubscribe linux-msdos" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [v6 PATCH 21/21] selftests/x86: Add tests for User-Mode Instruction Prevention
On Tue, Mar 7, 2017 at 4:32 PM, Ricardo Neri wrote: > Certain user space programs that run on virtual-8086 mode may utilize > instructions protected by the User-Mode Instruction Prevention (UMIP) > security feature present in new Intel processors: SGDT, SIDT and SMSW. In > such a case, a general protection fault is issued if UMIP is enabled. When > such a fault happens, the kernel catches it and emulates the results of > these instructions with dummy values. The purpose of this new > test is to verify whether the impacted instructions can be executed without > causing such #GP. If no #GP exceptions occur, we expect to exit virtual- > 8086 mode from INT 0x80. > > The instructions protected by UMIP are executed in representative use > cases: > a) the memory address of the result is given in the form of a displacement > from the base of the data segment > b) the memory address of the result is given in a general purpose register > c) the result is stored directly in a general purpose register. > > Unfortunately, it is not possible to check the results against a set of > expected values because no emulation will occur in systems that do not have > the UMIP feature. Instead, results are printed for verification. You could pre-initialize the result buffer to a bunch of non-matching values (1, 2, 3, ...) and then check that all the invocations of the same instruction gave the same value. If you do this, maybe make it a follow-up patch -- see other email. -- To unsubscribe from this list: send the line "unsubscribe linux-msdos" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [v5 17/20] x86/umip: Force a page fault when unable to copy emulated result to user
On Fri, Mar 3, 2017 at 1:41 PM, Ricardo Neri
wrote:
> fixup_umip_exception will be called from do_general_protection. If the
> former returns false, the latter will issue a SIGSEGV with SEND_SIG_PRIV.
> However, when emulation is successful but the emulated result cannot be
> copied to user space memory, it is more accurate to issue a SIGSEGV with
> SEGV_MAPERR with the offending address. A new function is inspired in
> force_sig_info_fault is introduced to model the page fault.
>
> Signed-off-by: Ricardo Neri
> ---
> arch/x86/kernel/umip.c | 45 +++--
> 1 file changed, 43 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/kernel/umip.c b/arch/x86/kernel/umip.c
> index e932f40..5b6a7cf 100644
> --- a/arch/x86/kernel/umip.c
> +++ b/arch/x86/kernel/umip.c
> @@ -170,6 +170,41 @@ static int __emulate_umip_insn(struct insn *insn, enum
> umip_insn umip_inst,
> }
>
> /**
> + * __force_sig_info_umip_fault - Force a SIGSEGV with SEGV_MAPERR
> + * @address: Address that caused the signal
> + * @regs: Register set containing the instruction pointer
> + *
> + * Force a SIGSEGV signal with SEGV_MAPERR as the error code. This function
> is
> + * intended to be used to provide a segmentation fault when the result of the
> + * UMIP emulation could not be copied to the user space memory.
> + *
> + * Return: none
> + */
> +static void __force_sig_info_umip_fault(void __user *address,
> + struct pt_regs *regs)
> +{
> + siginfo_t info;
> + struct task_struct *tsk = current;
> +
> + if (show_unhandled_signals && unhandled_signal(tsk, SIGSEGV)) {
> + printk_ratelimited("%s[%d] umip emulation segfault ip:%lx
> sp:%lx error:%lx in %lx\n",
> + tsk->comm, task_pid_nr(tsk), regs->ip,
> + regs->sp, UMIP_PF_USER | UMIP_PF_WRITE,
> + regs->ip);
> + }
> +
> + tsk->thread.cr2 = (unsigned long)address;
> + tsk->thread.error_code = UMIP_PF_USER | UMIP_PF_WRITE;
Please just move enum x86_pf_error_code into a header and rename the
fields X86_PF_USER, etc rather than duplicating it.
--Andy
--
To unsubscribe from this list: send the line "unsubscribe linux-msdos" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 15/17] x86/traps: Fixup general protection faults caused by UMIP
On Thu, Feb 23, 2017 at 2:15 PM, Ricardo Neri wrote: > On Thu, 2017-02-23 at 10:27 +0100, Peter Zijlstra wrote: >> On Wed, Feb 22, 2017 at 10:37:04PM -0800, Ricardo Neri wrote: >> > @@ -492,6 +493,9 @@ do_general_protection(struct pt_regs *regs, long >> > error_code) >> > RCU_LOCKDEP_WARN(!rcu_is_watching(), "entry code didn't wake RCU"); >> > cond_local_irq_enable(regs); >> > >> > + if (user_mode(regs) && (fixup_umip_exception(regs) == true)) >> > + return; >> >> I'm thinking >> >> if (user_mode(regs) && fixup_umip_exception(regs)) >> return; >> >> is actually easier to read. > > In a previous version Andy Lutomirsky suggested that > if (user_mode(regs) && (fixup_umip_exception(regs) == 0)) > > was easier to read :). Although at the time fixup_umip_exception > returned a numeric value. Now it only returns true/false for > successful/failed emulation. If with true/false not comparing to true > makes it easier to read, I will make the change. I think == true is silly :) --Andy -- To unsubscribe from this list: send the line "unsubscribe linux-msdos" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [v3 PATCH 05/10] x86/insn-kernel: Add support to resolve 16-bit addressing encodings
On Wed, Jan 25, 2017 at 9:50 PM, Ricardo Neri wrote: > On Wed, 2017-01-25 at 13:58 -0800, Andy Lutomirski wrote: >> On Wed, Jan 25, 2017 at 12:23 PM, Ricardo Neri >> wrote: >> > Tasks running in virtual-8086 mode will use 16-bit addressing form >> > encodings as described in the Intel 64 and IA-32 Architecture Software >> > Developer's Manual Volume 2A Section 2.1.5. 16-bit addressing encodings >> > differ in several ways from the 32-bit/64-bit addressing form encodings: >> > the r/m part of the ModRM byte points to different registers and, in some >> > cases, addresses can be indicated by the addition of the value of two >> > registers. Also, there is no support for SiB bytes. Thus, a separate >> > function is needed to parse this form of addressing. >> > >> > Furthermore, virtual-8086 mode tasks will use real-mode addressing. This >> > implies that the segment selectors do not point to a segment descriptor >> > but are used to compute logical addresses. Hence, there is a need to >> > add support to compute addresses using the segment selectors. If segment- >> > override prefixes are present in the instructions, they take precedence. >> > >> > Lastly, it is important to note that when a tasks is running in virtual- >> > 8086 mode and an interrupt/exception occurs, the CPU pushes to the stack >> > the segment selectors for ds, es, fs and gs. These are accesible via the >> > struct kernel_vm86_regs rather than pt_regs. >> > >> > Code for 16-bit addressing encodings is likely to be used only by virtual- >> > 8086 mode tasks. Thus, this code is wrapped to be built only if the >> > option CONFIG_VM86 is selected. >> >> That's not true. It's used in 16-bit protected mode, too. And there >> are (ugh!) six possibilities: > > Thanks for the clarification. I will enable the decoding of addresses > for 16-bit as well... and test the emulation code. >> >> - Normal 32-bit protected mode. This should already work. >> - Normal 64-bit protected mode. This should also already work. (I >> forget whether a 16-bit SS is either illegal or has no effect in this >> case.) > > For these two cases I am just taking the effective address that the user > space application provides, given that the segment selectors were set > beforehand (and with a base of 0). What do you mean by the base being zero? User code can set a nonzero DS base if it wants. In 64-bit mode (user_64bit_mode(regs)), the base is ignored unless there's an fs or gs prefix, and in 32-bit mode the base is never ignored. > >> - Virtual 8086 mode > > In this case I calculate the linear address as: > (segment_select << 4) + effective address. > >> - Normal 16-bit protected mode, used by DOSEMU and Wine. (16-bit CS, >> 16-bit address segment) >> - 16-bit CS, 32-bit address segment. IIRC this might be used by some >> 32-bit DOS programs to call BIOS. >> - 32-bit CS, 16-bit address segment. I don't know whether anything uses >> this. > > In all these protected modes, are you referring to the size in bits of > the base address of in the descriptor selected in the CS register? In > such a case I would need to get the base address and add it to the > effective address given in the operands of the instructions, right? No, I'm referring to the D/B bit. I'm a bit fuzzy on exactly how the instruction encoding works, but I think that 16-bit x86 code is encoded just like real mode code except that the selectors are used for real. >> size, but I suspect you'll need to handle 16-bit CS. > > Unless I am missing what is special with the 16-bit base address, I only > would need to add that base address to whatever effective address (aka, > offset) is encoded in the ModRM and displacement bytes. Exactly. (And make sure the instruction decoder can decode 16-bit instructions correctly.) -- To unsubscribe from this list: send the line "unsubscribe linux-msdos" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [v3 PATCH 05/10] x86/insn-kernel: Add support to resolve 16-bit addressing encodings
On Wed, Jan 25, 2017 at 12:23 PM, Ricardo Neri wrote: > Tasks running in virtual-8086 mode will use 16-bit addressing form > encodings as described in the Intel 64 and IA-32 Architecture Software > Developer's Manual Volume 2A Section 2.1.5. 16-bit addressing encodings > differ in several ways from the 32-bit/64-bit addressing form encodings: > the r/m part of the ModRM byte points to different registers and, in some > cases, addresses can be indicated by the addition of the value of two > registers. Also, there is no support for SiB bytes. Thus, a separate > function is needed to parse this form of addressing. > > Furthermore, virtual-8086 mode tasks will use real-mode addressing. This > implies that the segment selectors do not point to a segment descriptor > but are used to compute logical addresses. Hence, there is a need to > add support to compute addresses using the segment selectors. If segment- > override prefixes are present in the instructions, they take precedence. > > Lastly, it is important to note that when a tasks is running in virtual- > 8086 mode and an interrupt/exception occurs, the CPU pushes to the stack > the segment selectors for ds, es, fs and gs. These are accesible via the > struct kernel_vm86_regs rather than pt_regs. > > Code for 16-bit addressing encodings is likely to be used only by virtual- > 8086 mode tasks. Thus, this code is wrapped to be built only if the > option CONFIG_VM86 is selected. That's not true. It's used in 16-bit protected mode, too. And there are (ugh!) six possibilities: - Normal 32-bit protected mode. This should already work. - Normal 64-bit protected mode. This should also already work. (I forget whether a 16-bit SS is either illegal or has no effect in this case.) - Virtual 8086 mode - Normal 16-bit protected mode, used by DOSEMU and Wine. (16-bit CS, 16-bit address segment) - 16-bit CS, 32-bit address segment. IIRC this might be used by some 32-bit DOS programs to call BIOS. - 32-bit CS, 16-bit address segment. I don't know whether anything uses this. I don't know if anything you're doing cares about SS's, DS's, etc. size, but I suspect you'll need to handle 16-bit CS. -- To unsubscribe from this list: send the line "unsubscribe linux-msdos" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [v2 5/7] x86: Add emulation code for UMIP instructions
On Thu, Dec 29, 2016 at 9:23 PM, Ricardo Neri wrote: > On Tue, 2016-12-27 at 16:48 -0800, Andy Lutomirski wrote: >> >> >> > + if (nr_copied > 0) >> >> > + return -EFAULT; >> >> >> >> This should be the only EFAULT case. >> > Should this be EFAULT event if the caller cares only about successful >> > (return 0) vs failed (return non-0) emulation? >> >> In theory this particular error would be a page fault not a general >> protection fault (in the UMIP off case). If you were emulating it >> extra carefully, you could change the signal accordingly. But, as I >> said, I really doubt this matters. > > If simple enough and for the sake of accuracy, I could try to issue the > page fault. It seems to me that this entitles calling > force_sig_info_fault in this particular case as opposed to the > force_sig_info(SIGSEGV, SEND_SIG_PRIV, tsk) that do_general_protection > calls. Sure. You could even do it by sending the signal in the emulation code and returning true. --Andy -- To unsubscribe from this list: send the line "unsubscribe linux-msdos" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [v2 5/7] x86: Add emulation code for UMIP instructions
On Tue, Dec 27, 2016 at 4:39 PM, Ricardo Neri
wrote:
> On Fri, 2016-12-23 at 18:11 -0800, Andy Lutomirski wrote:
>> On Fri, Dec 23, 2016 at 5:37 PM, Ricardo Neri
>> wrote:
>> > The feature User-Mode Instruction Prevention present in recent Intel
>> > processor prevents a group of instructions from being executed with
>> > CPL > 0. Otherwise, a general protection fault is issued.
>> >
>> > Rather than relaying this fault to the user space (in the form of a SIGSEGV
>> > signal), the instructions protected by UMIP can be emulated to provide
>> > dummy results. This allows to conserve the current kernel behavior and not
>> > reveal the system resources that UMIP intends to protect (the global
>> > descriptor and interrupt descriptor tables, the segment selectors of the
>> > local descriptor table and the task state and the machine status word).
>> >
>> > This emulation is needed because certain applications (e.g., WineHQ) rely
>> > on this subset of instructions to function.
>> >
>> > The instructions protected by UMIP can be split in two groups. Those who
>> > return a kernel memory address (sgdt and sidt) and those who return a
>> > value (sldt, str and smsw).
>> >
>> > For the instructions that return a kernel memory address, the result is
>> > emulated as the location of a dummy variable in the kernel memory space.
>> > This is needed as applications such as WineHQ rely on the result being
>> > located in the kernel memory space function. The limit for the GDT and the
>> > IDT are set to zero.
>>
>> Nak. This is a trivial KASLR bypass. Just give them hardcoded
>> values. For x86_64, I would suggest 0xfffe and
>> 0x.
>
> I see. I assume you are suggesting these values for x86_64 because they
> lie in an unused hole. That makes sense to me.
>
> For the case of x86_32, I have trouble finding a suitable place as there
> are not many available holes. It could be put before VMALLOC_START or
> after VMALLOC_END but this would reveal the position of the vmalloc
> area. Although, to my knowledge, randomized memory is not available for
> x86_32. Without randomization, does it hurt to make sidt/sgdt return the
> address of a kernel static variable?
I would just use the same addresses, truncated. There's no reason
that the address needs to be truly not present -- it just needs to be
inaccessible to user code. Anything near the top of the address space
should work.
>
>>
>> >
>> > The instructions sldt and str return a segment selector relative to the
>> > base address of the global descriptor table. Since the actual address of
>> > such table is not revealed, it makes sense to emulate the result as zero.
>>
>> Hmm, now I wonder if anything uses SLDT to see if there is an LDT. If
>> so, we could emulate it better, but I doubt this matters.
>
> So you are saying that the emulated sldt should return a different value
> based on the presence/absence of a LDT? This could reveal this very
> fact.
User code knows whether the LDT exists because an LDT only exists if
the program called modify_ldt(). But I doubt this matters in
practice.
>> > +static int __emulate_umip_insn(struct insn *insn, enum umip_insn
>> > umip_inst,
>> > + unsigned char *data, int *data_size)
>> > +{
>> > + unsigned long const *dummy_base_addr;
>> > + unsigned short dummy_limit = 0;
>> > + unsigned short dummy_value = 0;
>> > +
>> > + switch (umip_inst) {
>> > + /*
>> > +* These two instructions return the base address and limit of the
>> > +* global and interrupt descriptor table. The base address can be
>> > +* 32-bit or 64-bit. Limit is always 16-bit.
>> > +*/
>> > + case UMIP_SGDT:
>> > + case UMIP_SIDT:
>> > + if (umip_inst == UMIP_SGDT)
>> > + dummy_base_addr = &umip_dummy_gdt_base;
>> > + else
>> > + dummy_base_addr = &umip_dummy_idt_base;
>> > + if (X86_MODRM_MOD(insn->modrm.value) == 3) {
>> > + WARN_ONCE(1, "SGDT cannot take register as
>> > argument!\n");
>>
>> No warnings please.
>
> I'll. Remove it.
Thanks. In general, WARN_ONCE, etc are supposed to indicate kernel
bugs, not user bugs.
>> > + int not_copied, nr_copied, reg_offset, dummy_data_size;
>> > + void
Re: [v2 5/7] x86: Add emulation code for UMIP instructions
On Fri, Dec 23, 2016 at 5:37 PM, Ricardo Neri
wrote:
> The feature User-Mode Instruction Prevention present in recent Intel
> processor prevents a group of instructions from being executed with
> CPL > 0. Otherwise, a general protection fault is issued.
>
> Rather than relaying this fault to the user space (in the form of a SIGSEGV
> signal), the instructions protected by UMIP can be emulated to provide
> dummy results. This allows to conserve the current kernel behavior and not
> reveal the system resources that UMIP intends to protect (the global
> descriptor and interrupt descriptor tables, the segment selectors of the
> local descriptor table and the task state and the machine status word).
>
> This emulation is needed because certain applications (e.g., WineHQ) rely
> on this subset of instructions to function.
>
> The instructions protected by UMIP can be split in two groups. Those who
> return a kernel memory address (sgdt and sidt) and those who return a
> value (sldt, str and smsw).
>
> For the instructions that return a kernel memory address, the result is
> emulated as the location of a dummy variable in the kernel memory space.
> This is needed as applications such as WineHQ rely on the result being
> located in the kernel memory space function. The limit for the GDT and the
> IDT are set to zero.
Nak. This is a trivial KASLR bypass. Just give them hardcoded
values. For x86_64, I would suggest 0xfffe and
0x.
>
> The instructions sldt and str return a segment selector relative to the
> base address of the global descriptor table. Since the actual address of
> such table is not revealed, it makes sense to emulate the result as zero.
Hmm, now I wonder if anything uses SLDT to see if there is an LDT. If
so, we could emulate it better, but I doubt this matters.
>
> The instruction smsw is emulated to return zero.
If you're going to emulate it, please return something plausible. The
protected mode bit should be on, for example. 0x33 is probably
reasonable.
> +static int __emulate_umip_insn(struct insn *insn, enum umip_insn umip_inst,
> + unsigned char *data, int *data_size)
> +{
> + unsigned long const *dummy_base_addr;
> + unsigned short dummy_limit = 0;
> + unsigned short dummy_value = 0;
> +
> + switch (umip_inst) {
> + /*
> +* These two instructions return the base address and limit of the
> +* global and interrupt descriptor table. The base address can be
> +* 32-bit or 64-bit. Limit is always 16-bit.
> +*/
> + case UMIP_SGDT:
> + case UMIP_SIDT:
> + if (umip_inst == UMIP_SGDT)
> + dummy_base_addr = &umip_dummy_gdt_base;
> + else
> + dummy_base_addr = &umip_dummy_idt_base;
> + if (X86_MODRM_MOD(insn->modrm.value) == 3) {
> + WARN_ONCE(1, "SGDT cannot take register as
> argument!\n");
No warnings please.
> +int fixup_umip_exception(struct pt_regs *regs)
> +{
> + struct insn insn;
> + unsigned char buf[MAX_INSN_SIZE];
> + /* 10 bytes is the maximum size of the result of UMIP instructions */
> + unsigned char dummy_data[10] = {0, 0, 0, 0, 0, 0, 0, 0, 0, 0};
> + int x86_64 = !test_thread_flag(TIF_IA32);
user_64bit_mode(regs)
> + int not_copied, nr_copied, reg_offset, dummy_data_size;
> + void __user *uaddr;
> + unsigned long *reg_addr;
> + enum umip_insn umip_inst;
> +
> + not_copied = copy_from_user(buf, (void __user *)regs->ip,
> sizeof(buf));
This is slightly wrong due to PKRU. I doubt we care.
> + nr_copied = sizeof(buf) - not_copied;
> + /*
> +* The decoder _should_ fail nicely if we pass it a short buffer.
> +* But, let's not depend on that implementation detail. If we
> +* did not get anything, just error out now.
> +*/
> + if (!nr_copied)
> + return -EFAULT;
If the caller cares about EINVAL vs EFAULT, it cares because it is
considering changing the signal to a fake page fault. If so, then
this should be EINVAL -- failure to read the text should just prevent
emulation.
> + insn_init(&insn, buf, nr_copied, x86_64);
> + insn_get_length(&insn);
> + if (nr_copied < insn.length)
> + return -EFAULT;
Ditto.
> +
> + umip_inst = __identify_insn(&insn);
> + /* Check if we found an instruction protected by UMIP */
> + if (umip_inst < 0)
> + return -EINVAL;
> +
> + if (__emulate_umip_insn(&insn, umip_inst, dummy_data,
> &dummy_data_size))
> + return -EINVAL;
> +
> + /* If operand is a register, write directly to it */
> + if (X86_MODRM_MOD(insn.modrm.value) == 3) {
> + reg_offset = get_reg_offset_rm(&insn, regs);
> + reg_addr = (unsigned long *)((unsigned long)regs +
> reg_offset);
> +
Re: [v2 6/7] x86/traps: Fixup general protection faults caused by UMIP
On Fri, Dec 23, 2016 at 5:37 PM, Ricardo Neri wrote: > If the User-Mode Instruction Prevention CPU feature is available and > enabled, a general protection fault will be issued if the instructions > sgdt, sldt, sidt, str or smsw are executed from user-mode context > (CPL > 0). If the fault was caused by any of the instructions protected > by UMIP, fixup_umip_exceptino will emulate dummy results for these > instructions. > > Cc: Andy Lutomirski > Cc: Andrew Morton > Cc: H. Peter Anvin > Cc: Borislav Petkov > Cc: Brian Gerst > Cc: Chen Yucong > Cc: Chris Metcalf > Cc: Dave Hansen > Cc: Fenghua Yu > Cc: Huang Rui > Cc: Jiri Slaby > Cc: Jonathan Corbet > Cc: Michael S. Tsirkin > Cc: Paul Gortmaker > Cc: Peter Zijlstra > Cc: Ravi V. Shankar > Cc: Shuah Khan > Cc: Vlastimil Babka > Cc: Tony Luck > Cc: Paolo Bonzini > Cc: Liang Z. Li > Cc: Alexandre Julliard > Cc: Stas Sergeev > Cc: x...@kernel.org > Cc: linux-msdos@vger.kernel.org > Signed-off-by: Ricardo Neri > --- > arch/x86/kernel/traps.c | 4 > 1 file changed, 4 insertions(+) > > diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c > index bf0c6d0..5044fb3 100644 > --- a/arch/x86/kernel/traps.c > +++ b/arch/x86/kernel/traps.c > @@ -64,6 +64,7 @@ > #include > #include > #include > +#include > > #ifdef CONFIG_X86_64 > #include > @@ -491,6 +492,9 @@ do_general_protection(struct pt_regs *regs, long > error_code) > RCU_LOCKDEP_WARN(!rcu_is_watching(), "entry code didn't wake RCU"); > cond_local_irq_enable(regs); > > + if (user_mode(regs) && !fixup_umip_exception(regs)) > + return; > + I would do fixup_umip_exception(regs) == 0 to make it more obvious what's going on. Also, since you're allowing this in v8086 mode, I think this should have an explicit test in tools/testing/selftests/x86/entry_from_vm86.c. I *think* it will work fine, but the pt_regs handling in vm86 mode is quite scary and has been rewritten recently. -- To unsubscribe from this list: send the line "unsubscribe linux-msdos" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [v2 1/7] x86/mpx: Do not use SIB index if index points to R/ESP
On Fri, Dec 23, 2016 at 5:37 PM, Ricardo Neri wrote: > Section 2.2.1.2 of the Intel 64 and IA-32 Architectures Software > Developer's Manual volume 2A states that when memory addressing is used > (i.e., mod part of ModR/M is not 3), a SIB byte is used and the index of > the SIB byte points to the R/ESP (i.e.,index = 4), the index should not be > used in the computation of the memory address. > > An example of such instruction could be > > insn -0x80(%rsp) > > This is represented as: > > [opcode] 4c 24 80 > > ModR/M: mod: 1, reg: 1: r/m: 4 (R/ESP) > SIB 24: sc: 0, index: 100 (R/ESP), base(R/ESP): 100 > Displacement -0x80 > > The correct address is (base) + displacement; no index is used. > > Care is taken to allow R12 to be used as index, which is a valid scenario. Since I have no idea what this patch has to do with the rest of the series, I'll ask a question: Why isn't this code in the standard x86 instruction decoder? Is the decoder similarly buggy? -- To unsubscribe from this list: send the line "unsubscribe linux-msdos" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [v2 2/7] x86/mpx: Fail when implicit zero-displacement is used along with R/EBP
On Fri, Dec 23, 2016 at 5:37 PM, Ricardo Neri
wrote:
> Section 2.2.1.2 of the Intel 64 and IA-32 Architectures Software
> Developer's Manual volume 2A states that when memory addressing with no
> explicit displacement (i.e, mod part of ModR/M is 0), a SIB byte is used
> and the base of the SIB byte points to (R/EBP) (i.e., base = 5), an
> explicit displacement of 0 must be used.
>
> Make the address decoder to return -EINVAL in such a case.
>
> Cc: Dave Hansen
> Cc: Adam Buchbinder
> Cc: Colin Ian King
> Cc: Lorenzo Stoakes
> Cc: Qiaowei Ren
> Cc: Ravi V. Shankar
> Cc: x...@kernel.org
> Signed-off-by: Ricardo Neri
> ---
> arch/x86/mm/mpx.c | 7 +++
> 1 file changed, 7 insertions(+)
>
> diff --git a/arch/x86/mm/mpx.c b/arch/x86/mm/mpx.c
> index 6a75a75..71681d0 100644
> --- a/arch/x86/mm/mpx.c
> +++ b/arch/x86/mm/mpx.c
> @@ -120,6 +120,13 @@ static int get_reg_offset(struct insn *insn, struct
> pt_regs *regs,
>
> case REG_TYPE_BASE:
> regno = X86_SIB_BASE(insn->sib.value);
> + if (regno == 5 && X86_MODRM_RM(insn->modrm.value) == 0) {
> + WARN_ONCE(1, "An explicit displacement is required
> when %sBP used as SIB base.",
> + (IS_ENABLED(CONFIG_X86_64) && insn->x86_64)
> ?
> + "R13 or R" : "E");
> + return -EINVAL;
> + }
> +
Now that I've read the cover letter, I see what's going on. This
should not warn -- user code can easily trigger this deliberately.
--
To unsubscribe from this list: send the line "unsubscribe linux-msdos" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
