Re: [PATCH v7 16/26] x86/insn-eval: Support both signed 32-bit and 64-bit effective addresses

2017-07-28 Thread Borislav Petkov
On Thu, Jul 27, 2017 at 07:04:52PM -0700, Ricardo Neri wrote: > However using the union could be less readable than having two almost > identical functions. So having some small duplication for the sake of clarity and readability is much better, if you ask me. And it's not like you're duplicating

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

2017-07-27 Thread Borislav Petkov
On Tue, Jul 25, 2017 at 05:44:08PM -0700, Ricardo Neri wrote: > On Fri, 2017-06-09 at 18:10 +0200, Borislav Petkov wrote: > > On Fri, May 05, 2017 at 11:17:22AM -0700, Ricardo Neri wrote: > > > User_mode Instruction Prevention (UMIP) is enabled by setting/clearing a

Re: [PATCH v7 16/26] x86/insn-eval: Support both signed 32-bit and 64-bit effective addresses

2017-07-27 Thread Borislav Petkov
On Tue, Jul 25, 2017 at 04:48:13PM -0700, Ricardo Neri wrote: > I meant to say the 4 most significant bytes. In this case, the > 64-address 0x1234 would lie in the kernel memory while > 0x1234 would correctly be in the user space memory. That explanation is better. > Yes, perhaps

Re: [PATCH v7 10/26] x86/insn-eval: Add utility functions to get segment selector

2017-06-19 Thread Borislav Petkov
On Thu, Jun 15, 2017 at 11:37:51AM -0700, Ricardo Neri wrote: > Wouldn't this be ending up mixing the actual segment register and > segment register overrides? I plan to have a function that parses the > segment override prefixes and returns SEG_REG_CS/DS/ES/FS/GS or > SEG_REG_IGNORE for long mode

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

2017-06-09 Thread Borislav Petkov
abled by adding clearcpuid=514 > to the kernel parameters. > > Cc: Andy Lutomirski <l...@kernel.org> > Cc: Andrew Morton <a...@linux-foundation.org> > Cc: H. Peter Anvin <h...@zytor.com> > Cc: Borislav Petkov <b...@suse.de> > Cc: Brian Gerst <brge...@gmail.com>

Re: [PATCH v7 23/26] x86/traps: Fixup general protection faults caused by UMIP

2017-06-09 Thread Borislav Petkov
IGSEGV signal is emitted. > > Please note that fixup_umip_exception also caters for the case when > the fault originated while running in virtual-8086 mode. > > Cc: Andy Lutomirski <l...@kernel.org> > Cc: Andrew Morton <a...@linux-foundation.org> > Cc: H. Peter Anvin &

Re: [PATCH v7 22/26] x86/umip: Force a page fault when unable to copy emulated result to user

2017-06-09 Thread Borislav Petkov
gt; SEGV_MAPERR with the offending address. > A new function is inspired in That reads funny. > force_sig_info_fault is introduced to model the page fault. > > Cc: Andy Lutomirski <l...@kernel.org> > Cc: Andrew Morton <a...@linux-foundation.org> > Cc: H. Peter Anvin <h...@zyto

Re: [PATCH v7 21/26] x86: Add emulation code for UMIP instructions

2017-06-08 Thread Borislav Petkov
ion uses local descriptor table. Btw, I could very well use all that nice explanation in umip.c too so that the high-level behavior is documented. > Cc: Andy Lutomirski <l...@kernel.org> > Cc: Andrew Morton <a...@linux-foundation.org> > Cc: H. Peter Anvin <h...@zytor.com> > Cc

Re: [PATCH v7 20/26] x86/cpufeature: Add User-Mode Instruction Prevention definitions

2017-06-07 Thread Borislav Petkov
hine Status Word > * STR - Store Task Register > > This feature is also added to the list of disabled-features to allow > a cleaner handling of build-time configuration. > > Cc: Andy Lutomirski <l...@kernel.org> > Cc: Andrew Morton <a...@linux-foundation.org> &

Re: [PATCH v7 18/26] x86/insn-eval: Add support to resolve 16-bit addressing encodings

2017-06-07 Thread Borislav Petkov
mhira...@kernel.org> > Cc: Adrian Hunter <adrian.hun...@intel.com> > Cc: Kees Cook <keesc...@chromium.org> > Cc: Thomas Garnier <thgar...@google.com> > Cc: Peter Zijlstra <pet...@infradead.org> > Cc: Borislav Petkov <b...@suse.de> > Cc: Dmitry Vyuko

Re: [PATCH v7 16/26] x86/insn-eval: Support both signed 32-bit and 64-bit effective addresses

2017-06-07 Thread Borislav Petkov
On Fri, May 05, 2017 at 11:17:14AM -0700, Ricardo Neri wrote: > @@ -697,18 +753,21 @@ void __user *insn_get_addr_ref(struct insn *insn, > struct pt_regs *regs) > { > unsigned long linear_addr, seg_base_addr, seg_limit; > long eff_addr, base, indx; > - int addr_offset,

Re: [PATCH v7 16/26] x86/insn-eval: Support both signed 32-bit and 64-bit effective addresses

2017-06-07 Thread Borislav Petkov
iaowei Ren <qiaowei@intel.com> > Cc: Arnaldo Carvalho de Melo <a...@redhat.com> > Cc: Masami Hiramatsu <mhira...@kernel.org> > Cc: Adrian Hunter <adrian.hun...@intel.com> > Cc: Kees Cook <keesc...@chromium.org> > Cc: Thomas Garnier <thgar...@google.com&

Re: [PATCH v7 14/26] x86/insn-eval: Indicate a 32-bit displacement if ModRM.mod is 0 and ModRM.rm is 5

2017-06-07 Thread Borislav Petkov
On Fri, May 05, 2017 at 11:17:12AM -0700, Ricardo Neri wrote: > Section 2.2.1.3 of the Intel 64 and IA-32 Architectures Software > Developer's Manual volume 2A states that when ModRM.mod is zero and > ModRM.rm is 101b, a 32-bit displacement follows the ModRM byte. This means > that none of the

Re: [PATCH v7 13/26] x86/insn-eval: Add function to get default params of code segment

2017-06-07 Thread Borislav Petkov
Masami Hiramatsu <mhira...@kernel.org> > Cc: Adrian Hunter <adrian.hun...@intel.com> > Cc: Kees Cook <keesc...@chromium.org> > Cc: Thomas Garnier <thgar...@google.com> > Cc: Peter Zijlstra <pet...@infradead.org> > Cc: Borislav Petkov <b...@suse.de> &

Re: [PATCH v7 07/26] x86/insn-eval: Do not BUG on invalid register type

2017-06-07 Thread Borislav Petkov
On Tue, Jun 06, 2017 at 05:28:52PM -0700, Ricardo Neri wrote: > I see. You were more concerned about the naming of the coding artifacts > (e.g., function names, error prints, etc) than the actual filenames. Well, I'm not sure here. We could either have a generalized prefix or put the function

Re: [PATCH v7 09/26] x86/insn-eval: Add utility function to identify string instructions

2017-06-06 Thread Borislav Petkov
On Mon, Jun 05, 2017 at 11:01:21PM -0700, Ricardo Neri wrote: > If I was to leave out string instructions from this function it should > be renamed as is_string_instruction_non_lods_outs. In my opinion this > separation makes the code more clear and I would end up having logic to > decide which

Re: [PATCH v7 07/26] x86/insn-eval: Do not BUG on invalid register type

2017-06-06 Thread Borislav Petkov
On Mon, Jun 05, 2017 at 11:06:58PM -0700, Ricardo Neri wrote: > I agree that insn-eval reads somewhat funny. I did not want to go with > insn-dec.c as insn.c, in my opinion, already decodes the instruction > (i.e., it finds prefixes, opcodes, ModRM, SIB and displacement bytes). > In insn-eval.c I

Re: [PATCH v7 12/26] x86/insn-eval: Add utility functions to get segment descriptor base address and limit

2017-05-31 Thread Borislav Petkov
On Fri, May 05, 2017 at 11:17:10AM -0700, Ricardo Neri wrote: > With segmentation, the base address of the segment descriptor is needed > to compute a linear address. The segment descriptor used in the address > computation depends on either any segment override prefixes in the > instruction or

Re: [PATCH v7 10/26] x86/insn-eval: Add utility functions to get segment selector

2017-05-30 Thread Borislav Petkov
com> > Cc: Lorenzo Stoakes <lstoa...@gmail.com> > Cc: Qiaowei Ren <qiaowei@intel.com> > Cc: Arnaldo Carvalho de Melo <a...@redhat.com> > Cc: Masami Hiramatsu <mhira...@kernel.org> > Cc: Adrian Hunter <adrian.hun...@intel.com> > Cc: Kees Cook &l

Re: [PATCH v7 09/26] x86/insn-eval: Add utility function to identify string instructions

2017-05-29 Thread Borislav Petkov
On Fri, May 05, 2017 at 11:17:07AM -0700, Ricardo Neri wrote: > String instructions are special because in protected mode, the linear > address is always obtained via the ES segment register in operands that > use the (E)DI register. ... and DS for rSI. If we're going to account for both

Re: [PATCH v7 08/26] x86/insn-eval: Add a utility function to get register offsets

2017-05-29 Thread Borislav Petkov
el.com> > Cc: Arnaldo Carvalho de Melo <a...@redhat.com> > Cc: Masami Hiramatsu <mhira...@kernel.org> > Cc: Adrian Hunter <adrian.hun...@intel.com> > Cc: Kees Cook <keesc...@chromium.org> > Cc: Thomas Garnier <thgar...@google.com> > Cc: Peter Zijlstra &l

Re: [PATCH v7 07/26] x86/insn-eval: Do not BUG on invalid register type

2017-05-29 Thread Borislav Petkov
> abused from user space programs, use the rate-limited variant of printk. > > Cc: Borislav Petkov <b...@suse.de> > Cc: Andy Lutomirski <l...@kernel.org> > Cc: Dave Hansen <dave.han...@linux.intel.com> > Cc: Adam Buchbinder <adam.buchbin...@gmail.com> &

Re: [PATCH v7 05/26] x86/mpx: Do not use SIB.base if its value is 101b and ModRM.mod = 0

2017-05-29 Thread Borislav Petkov
they should ignore the base. > > Cc: Borislav Petkov <b...@suse.de> > Cc: Andy Lutomirski <l...@kernel.org> > Cc: Dave Hansen <dave.han...@linux.intel.com> > Cc: Adam Buchbinder <adam.buchbin...@gmail.com> > Cc: Colin Ian King <colin.k...@canonical.co

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

2017-05-27 Thread Borislav Petkov
On Fri, May 26, 2017 at 08:40:26PM -0700, Ricardo Neri wrote: > This change was initially intended to only rename the error codes, > without functional changes. Would making change be considered a change > in functionality? How? The before-and-after asm should be the identical. --

Re: [PATCH v7 04/26] x86/mpx: Do not use SIB.index if its value is 100b and ModRM.mod is not 11b

2017-05-24 Thread Borislav Petkov
b][reg:1b][rm:100b] >SIB: 0x23 [scale:0b][.X: 1b, index:100b][.B:0b, base:11b] >Displacement: 0x80 (1-byte, as per ModRM.mod = 1b) > > Cc: Borislav Petkov <b...@suse.de> > Cc: Andy Lutomirski <l...@kernel.org> > Cc: Dave Hansen <dave.han...

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

2017-05-21 Thread Borislav Petkov
if (unlikely(!down_read_trylock(>mmap_sem))) { > - if ((error_code & PF_USER) == 0 && > + if ((error_code & X86_PF_USER) == 0 && if (!(error_code & X86_PF_USER)) With that fixed: Reviewed-by: Borislav Petkov <b...@suse.de> -- Re

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

2017-05-21 Thread Borislav Petkov
_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 <b...@sus

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

2017-05-15 Thread Borislav Petkov
On Thu, May 11, 2017 at 07:13:57PM -0700, Ricardo Neri wrote: > Sure I can. Would this trigger a v8 of my series? I was hoping v7 series > could be merged and then start doing incremental work on top of it. Does > it make sense? I guess that's tip guys' call. -- Regards/Gruss, Boris. SUSE

Re: [v6 PATCH 12/21] x86/insn: Support both signed 32-bit and 64-bit effective addresses

2017-05-08 Thread Borislav Petkov
On Wed, Apr 26, 2017 at 08:33:46PM -0700, Ricardo Neri wrote: > This is the reason I check the value of long_bytes. If long_bytes is not > 4, being the only other possible value 8 (perhaps I need to issue an > error when the value is not any of these values), Well, maybe I'm a bit too paranoid.

Re: [v6 PATCH 10/21] x86/insn-eval: Do not use R/EBP as base if mod in ModRM is zero

2017-05-07 Thread Borislav Petkov
On Wed, Apr 26, 2017 at 06:29:59PM -0700, Ricardo Neri wrote: > > if (X86_MODRM_MOD(insn->modrm.value) == 0 && > > X86_MODRM_RM(insn->modrm.value) == 5) > > > > looks more understandable to me. > > Should I go with !(X86_MODRM_MOD(insn->modrm.value)) as you suggested in > other

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

2017-05-05 Thread 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

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

2017-05-05 Thread 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.

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

2017-05-04 Thread Borislav Petkov
On Wed, Apr 26, 2017 at 02:51:56PM -0700, Ricardo Neri wrote: > > > + seg >= current->active_mm->context.ldt->size)) { > > > > ldt->size is the size of the descriptor table but you've shifted seg by > > 3. That selector index is shifted by 3 (to the left) to form an offset >

Re: [v6 PATCH 06/21] x86/insn-eval: Add utility functions to get segment selector

2017-04-30 Thread Borislav Petkov
On Wed, Apr 26, 2017 at 01:44:43PM -0700, Ricardo Neri wrote: > I regard that the role of this function is to obtain the the segment > selector from either of the prefixes or inferred from the operands. It > is the role of caller to determine if the segment selector should be > ignored. No, this

Re: [v6 PATCH 05/21] x86/insn-eval: Add utility functions to get register offsets

2017-04-28 Thread Borislav Petkov
On Wed, Apr 26, 2017 at 11:13:44AM -0700, Ricardo Neri wrote: > Masami Hiramatsu had originally requested to add the two functions. I > suppose the unneeded functions could be added if/when needed. Yap, exactly. -- Regards/Gruss, Boris. SUSE Linux GmbH, GF: Felix Imendörffer, Jane

Re: [v6 PATCH 03/21] x86/mpx: Do not use R/EBP as base in the SIB byte with Mod = 0

2017-04-26 Thread Borislav Petkov
On Tue, Apr 25, 2017 at 07:04:20PM -0700, Ricardo Neri wrote: > For the specific case of ModRM.mod being 0, I feel I need to clarify > that REX.B is not decoded and if SIB.base is %r13 the base is also 0. Well, that all doesn't matter. The rule is this: ModRM.mod == 00b and ModRM.r/m == 101b ->

Re: [v6 PATCH 12/21] x86/insn: Support both signed 32-bit and 64-bit effective addresses

2017-04-25 Thread Borislav Petkov
wei@intel.com> > Cc: Arnaldo Carvalho de Melo <a...@redhat.com> > Cc: Masami Hiramatsu <mhira...@kernel.org> > Cc: Adrian Hunter <adrian.hun...@intel.com> > Cc: Kees Cook <keesc...@chromium.org> > Cc: Thomas Garnier <thgar...@google.com> > Cc: Peter

Re: [v6 PATCH 11/21] insn/eval: Incorporate segment base in address computation

2017-04-21 Thread Borislav Petkov
On Tue, Mar 07, 2017 at 04:32:44PM -0800, Ricardo Neri wrote: > insn_get_addr_ref returns the effective address as defined by the Please end function names with parentheses. > section 3.7.5.1 Vol 1 of the Intel 64 and IA-32 Architectures Software > Developer's Manual. In order to compute the

Re: [v6 PATCH 10/21] x86/insn-eval: Do not use R/EBP as base if mod in ModRM is zero

2017-04-21 Thread Borislav Petkov
il.com> > Cc: Qiaowei Ren <qiaowei@intel.com> > Cc: Arnaldo Carvalho de Melo <a...@redhat.com> > Cc: Masami Hiramatsu <mhira...@kernel.org> > Cc: Adrian Hunter <adrian.hun...@intel.com> > Cc: Kees Cook <keesc...@chromium.org> > Cc: Thomas Garn

Re: [v6 PATCH 09/21] x86/insn-eval: Add functions to get default operand and address sizes

2017-04-20 Thread Borislav Petkov
ho de Melo <a...@redhat.com> > Cc: Masami Hiramatsu <mhira...@kernel.org> > Cc: Adrian Hunter <adrian.hun...@intel.com> > Cc: Kees Cook <keesc...@chromium.org> > Cc: Thomas Garnier <thgar...@google.com> > Cc: Peter Zijlstra <pet...@infradead.org> &g

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

2017-04-20 Thread Borislav Petkov
n <qiaowei@intel.com> > Cc: Arnaldo Carvalho de Melo <a...@redhat.com> > Cc: Masami Hiramatsu <mhira...@kernel.org> > Cc: Adrian Hunter <adrian.hun...@intel.com> > Cc: Kees Cook <keesc...@chromium.org> > Cc: Thomas Garnier <thgar...@google.com&

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

2017-04-19 Thread Borislav Petkov
com> > Cc: Kees Cook <keesc...@chromium.org> > Cc: Thomas Garnier <thgar...@google.com> > Cc: Peter Zijlstra <pet...@infradead.org> > Cc: Borislav Petkov <b...@suse.de> > Cc: Dmitry Vyukov <dvyu...@google.com> > Cc: Ravi V. Shanka

Re: [v6 PATCH 06/21] x86/insn-eval: Add utility functions to get segment selector

2017-04-18 Thread Borislav Petkov
Arnaldo Carvalho de Melo <a...@redhat.com> > Cc: Masami Hiramatsu <mhira...@kernel.org> > Cc: Adrian Hunter <adrian.hun...@intel.com> > Cc: Kees Cook <keesc...@chromium.org> > Cc: Thomas Garnier <thgar...@google.com> > Cc: Peter Zijlstra <pet...@infrade

Re: [v6 PATCH 05/21] x86/insn-eval: Add utility functions to get register offsets

2017-04-12 Thread Borislav Petkov
;a...@redhat.com> > Cc: Masami Hiramatsu <mhira...@kernel.org> > Cc: Adrian Hunter <adrian.hun...@intel.com> > Cc: Kees Cook <keesc...@chromium.org> > Cc: Thomas Garnier <thgar...@google.com> > Cc: Peter Zijlstra <pet...@infradead.org> > Cc: Borislav P

Re: [v6 PATCH 04/21] x86/mpx, x86/insn: Relocate insn util functions to a new insn-kernel

2017-04-12 Thread Borislav Petkov
On Tue, Mar 07, 2017 at 04:32:37PM -0800, Ricardo Neri wrote: > Other kernel submodules can benefit from using the utility functions > defined in mpx.c to obtain the addresses and values of operands contained > in the general purpose registers. An instance of this is the emulation code > used for

Re: [v6 PATCH 03/21] x86/mpx: Do not use R/EBP as base in the SIB byte with Mod = 0

2017-04-11 Thread Borislav Petkov
On Tue, Mar 07, 2017 at 04:32:36PM -0800, 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 a SIB byte is used and the > base of the SIB byte points to R/EBP (i.e., base = 5) and the mod part > of the ModRM byte

Re: [v6 PATCH 01/21] x86/mpx: Use signed variables to compute effective addresses

2017-04-11 Thread Borislav Petkov
On Tue, Mar 07, 2017 at 04:32:34PM -0800, Ricardo Neri wrote: > Even though memory addresses are unsigned. The operands used to compute the ... unsigned, the operands ... > effective address do have a sign. This is true for the r/m part of the > ModRM byte, the