[v6 PATCH 08/21] x86/insn-eval: Add utility function to get segment descriptor base address
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 in the instruction or the default segment determined by the registers involved in the address computation. Thus, both the instruction as well as the register (specified as the offset from the base of pt_regs) are given as inputs, along with a boolean variable to select between override and default. The segment selector is determined by get_seg_selector with the inputs described above. Once the selector is known the base address is determined. In protected mode, the selector is used to obtain the segment descriptor and then its base address. If in 64-bit user mode, the segment = base address is zero except when FS or GS are used. In virtual-8086 mode, the base address is computed as the value of the segment selector shifted 4 positions to the left. Cc: Dave Hansen Cc: Adam Buchbinder Cc: Colin Ian King Cc: Lorenzo Stoakes Cc: Qiaowei Ren Cc: Arnaldo Carvalho de Melo Cc: Masami Hiramatsu Cc: Adrian Hunter Cc: Kees Cook Cc: Thomas Garnier Cc: Peter Zijlstra Cc: Borislav Petkov Cc: Dmitry Vyukov Cc: Ravi V. Shankar Cc: x...@kernel.org Signed-off-by: Ricardo Neri --- arch/x86/include/asm/insn-eval.h | 2 ++ arch/x86/lib/insn-eval.c | 66 2 files changed, 68 insertions(+) diff --git a/arch/x86/include/asm/insn-eval.h b/arch/x86/include/asm/insn-eval.h index 754211b..b201742 100644 --- a/arch/x86/include/asm/insn-eval.h +++ b/arch/x86/include/asm/insn-eval.h @@ -15,5 +15,7 @@ void __user *insn_get_addr_ref(struct insn *insn, struct pt_regs *regs); int insn_get_reg_offset_modrm_rm(struct insn *insn, struct pt_regs *regs); int insn_get_reg_offset_sib_base(struct insn *insn, struct pt_regs *regs); int insn_get_reg_offset_sib_base(struct insn *insn, struct pt_regs *regs); +unsigned long insn_get_seg_base(struct pt_regs *regs, struct insn *insn, + int regoff, bool use_default_seg); #endif /* _ASM_X86_INSN_EVAL_H */ diff --git a/arch/x86/lib/insn-eval.c b/arch/x86/lib/insn-eval.c index 8608adf..383ca83 100644 --- a/arch/x86/lib/insn-eval.c +++ b/arch/x86/lib/insn-eval.c @@ -355,6 +355,72 @@ static int get_desc(unsigned short seg, struct desc_struct **desc) } /** + * insn_get_seg_base() - Obtain base address contained in descriptor + * @regs: Set of registers containing the segment selector + * @insn: Instruction structure with selector override prefixes + * @regoff:Operand offset, in pt_regs, of which the selector is needed + * @use_default_seg: Use the default segment instead of prefix overrides + * + * Obtain the base address of the segment descriptor as indicated by either + * any segment override prefixes contained in insn or the default segment + * applicable to the register indicated by regoff. regoff is specified as the + * offset in bytes from the base of pt_regs. + * + * Return: In protected mode, base address of the segment. It may be zero in + * certain cases for 64-bit builds and/or 64-bit applications. In virtual-8086 + * mode, the segment selector shifed 4 positions to the right. -1L in case of + * error. + */ +unsigned long insn_get_seg_base(struct pt_regs *regs, struct insn *insn, + int regoff, bool use_default_seg) +{ + struct desc_struct *desc; + unsigned short seg; + enum segment seg_type; + int ret; + + seg_type = resolve_seg_selector(insn, regoff, use_default_seg); + + seg = get_segment_selector(regs, seg_type); + if (seg < 0) + return -1L; + + if (v8086_mode(regs)) + /* +* Base is simply the segment selector shifted 4 +* positions to the right. +*/ + return (unsigned long)(seg << 4); + +#ifdef CONFIG_X86_64 + if (user_64bit_mode(regs)) { + /* +* Only FS or GS will have a base address, the rest of +* the segments' bases are forced to 0. +*/ + unsigned long base; + + if (seg_type == SEG_FS) + rdmsrl(MSR_FS_BASE, base); + else if (seg_type == SEG_GS) + /* +* swapgs was called at the kernel entry point. Thus, +* MSR_KERNEL_GS_BASE will have the user-space GS base. +*/ + rdmsrl(MSR_KERNEL_GS_BASE, base); + else + base = 0; + return base; + } +#endif + ret = get_desc(seg, &desc); + if (ret) + return -1L; + + return get_desc_base(desc); +} + +/** * insn_get_reg_offset_modrm_rm - Obtain register in r/m part of ModRM byte * @insn: Instruction structure containing the ModRM
Re: [v6 PATCH 08/21] x86/insn-eval: Add utility function to get segment descriptor base address
On Fri, 2017-05-05 at 19:28 +0200, Borislav Petkov wrote: > 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. Yes, in my v7 I ignore the segment register if we are in long mode [1]. > > > 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. In my v7, I have added a section my function resolve_seg_register() that ignores segment overrides if it sees string instructions and the register EDI and defaults to ES. If the register is EIP, it defaults to CS. To determine if an instruction is a string instruction I do check for the size of the opcode and the opcodes that you mention plus others based on the Intel Software Development Manual[2]. [1]. https://lkml.org/lkml/2017/5/5/405 [2]. https://lkml.org/lkml/2017/5/5/410 Thanks and BR, Ricardo > > -- > Regards/Gruss, > Boris. > > SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB > 21284 (AG Nürnberg)
Re: [v6 PATCH 08/21] x86/insn-eval: Add utility function to get segment descriptor base address
On Fri, 2017-05-05 at 19:19 +0200, Borislav Petkov wrote: > 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. In my v7, I simply named my enumeration enum segment_register, which is what they are. Some of its entries happen to have the value of the segment override prefixes but also have special entries as SEG_REG_INVAL when for errors and SEG_REG_IGNORE for long mode [1]. Thanks and BR, Ricardo [1]. https://lkml.org/lkml/2017/5/5/405
Re: [v6 PATCH 08/21] x86/insn-eval: Add utility function to get segment descriptor base address
On Tue, Mar 07, 2017 at 04:32:41PM -0800, 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 in the s/in the // > instruction or the default segment determined by the registers involved > in the address computation. Thus, both the instruction as well as the > register (specified as the offset from the base of pt_regs) are given as > inputs, along with a boolean variable to select between override and > default. > > The segment selector is determined by get_seg_selector with the inputs Please end function names with parentheses: get_seg_selector(). > described above. Once the selector is known the base address is known, ... > determined. In protected mode, the selector is used to obtain the segment > descriptor and then its base address. If in 64-bit user mode, the segment = > base address is zero except when FS or GS are used. In virtual-8086 mode, > the base address is computed as the value of the segment selector shifted 4 > positions to the left. Good. > Cc: Dave Hansen > Cc: Adam Buchbinder > Cc: Colin Ian King > Cc: Lorenzo Stoakes > Cc: Qiaowei Ren > Cc: Arnaldo Carvalho de Melo > Cc: Masami Hiramatsu > Cc: Adrian Hunter > Cc: Kees Cook > Cc: Thomas Garnier > Cc: Peter Zijlstra > Cc: Borislav Petkov > Cc: Dmitry Vyukov > Cc: Ravi V. Shankar > Cc: x...@kernel.org > Signed-off-by: Ricardo Neri > --- > arch/x86/include/asm/insn-eval.h | 2 ++ > arch/x86/lib/insn-eval.c | 66 > > 2 files changed, 68 insertions(+) > > diff --git a/arch/x86/include/asm/insn-eval.h > b/arch/x86/include/asm/insn-eval.h > index 754211b..b201742 100644 > --- a/arch/x86/include/asm/insn-eval.h > +++ b/arch/x86/include/asm/insn-eval.h > @@ -15,5 +15,7 @@ void __user *insn_get_addr_ref(struct insn *insn, struct > pt_regs *regs); > int insn_get_reg_offset_modrm_rm(struct insn *insn, struct pt_regs *regs); > int insn_get_reg_offset_sib_base(struct insn *insn, struct pt_regs *regs); > int insn_get_reg_offset_sib_base(struct insn *insn, struct pt_regs *regs); > +unsigned long insn_get_seg_base(struct pt_regs *regs, struct insn *insn, > + int regoff, bool use_default_seg); > > #endif /* _ASM_X86_INSN_EVAL_H */ > diff --git a/arch/x86/lib/insn-eval.c b/arch/x86/lib/insn-eval.c > index 8608adf..383ca83 100644 > --- a/arch/x86/lib/insn-eval.c > +++ b/arch/x86/lib/insn-eval.c > @@ -355,6 +355,72 @@ static int get_desc(unsigned short seg, struct > desc_struct **desc) > } > > /** > + * insn_get_seg_base() - Obtain base address contained in descriptor > + * @regs:Set of registers containing the segment selector > + * @insn:Instruction structure with selector override prefixes > + * @regoff: Operand offset, in pt_regs, of which the selector is needed > + * @use_default_seg: Use the default segment instead of prefix overrides I'm wondering whether you really need that bool or you can deduce this from pt_regs... I guess I'll see... > + * > + * Obtain the base address of the segment descriptor as indicated by either > + * any segment override prefixes contained in insn or the default segment > + * applicable to the register indicated by regoff. regoff is specified as the > + * offset in bytes from the base of pt_regs. > + * > + * Return: In protected mode, base address of the segment. It may be zero in > + * certain cases for 64-bit builds and/or 64-bit applications. In > virtual-8086 > + * mode, the segment selector shifed 4 positions to the right. -1L in case of s/shifed/shifted/ > + * error. > + */ > +unsigned long insn_get_seg_base(struct pt_regs *regs, struct insn *insn, > + int regoff, bool use_default_seg) > +{ > + struct desc_struct *desc; > + unsigned short seg; > + enum segment seg_type; > + int ret; > + > + seg_type = resolve_seg_selector(insn, regoff, use_default_seg); <--- error handling. And that's not really a "seg_type" but simply the "sel"-ector. And that "enum segment" is not really a segment but an segment override prefixes enum. Can we please get the nomenclature right first? > + > + seg = get_segment_selector(regs, seg_type); s/seg/sel/ > + if (seg < 0) > + return -1L; > + > + if (v8086_mode(regs)) > + /* > + * Base is simply the segment selector shifted 4 > + * positions to the right. > + */ > + return (unsigned long)(seg << 4); > + > +#ifdef CONFIG_X86_64 > + if (user_64bit_mode(regs)) { if (IS_ENABLED(CONFIG_X86_64) && user_64bit_mode(regs)) { > + /* > + * Only FS or GS will have a base address, the rest of > + * the segments' bases are forced to 0. > + */ > +
Re: [v6 PATCH 08/21] x86/insn-eval: Add utility function to get segment descriptor base address
On Thu, 2017-04-20 at 10:25 +0200, Borislav Petkov wrote: > On Tue, Mar 07, 2017 at 04:32:41PM -0800, 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 in the > > s/in the // I will fix this typo. > > > instruction or the default segment determined by the registers involved > > in the address computation. Thus, both the instruction as well as the > > register (specified as the offset from the base of pt_regs) are given as > > inputs, along with a boolean variable to select between override and > > default. > > > > The segment selector is determined by get_seg_selector with the inputs > > Please end function names with parentheses: get_seg_selector(). I will use parentheses. > > > described above. Once the selector is known the base address is > > known, ... Will fix. > > > determined. In protected mode, the selector is used to obtain the segment > > descriptor and then its base address. If in 64-bit user mode, the segment = > > base address is zero except when FS or GS are used. In virtual-8086 mode, > > the base address is computed as the value of the segment selector shifted 4 > > positions to the left. > > Good. > > > Cc: Dave Hansen > > Cc: Adam Buchbinder > > Cc: Colin Ian King > > Cc: Lorenzo Stoakes > > Cc: Qiaowei Ren > > Cc: Arnaldo Carvalho de Melo > > Cc: Masami Hiramatsu > > Cc: Adrian Hunter > > Cc: Kees Cook > > Cc: Thomas Garnier > > Cc: Peter Zijlstra > > Cc: Borislav Petkov > > Cc: Dmitry Vyukov > > Cc: Ravi V. Shankar > > Cc: x...@kernel.org > > Signed-off-by: Ricardo Neri > > --- > > arch/x86/include/asm/insn-eval.h | 2 ++ > > arch/x86/lib/insn-eval.c | 66 > > > > 2 files changed, 68 insertions(+) > > > > diff --git a/arch/x86/include/asm/insn-eval.h > > b/arch/x86/include/asm/insn-eval.h > > index 754211b..b201742 100644 > > --- a/arch/x86/include/asm/insn-eval.h > > +++ b/arch/x86/include/asm/insn-eval.h > > @@ -15,5 +15,7 @@ void __user *insn_get_addr_ref(struct insn *insn, struct > > pt_regs *regs); > > int insn_get_reg_offset_modrm_rm(struct insn *insn, struct pt_regs *regs); > > int insn_get_reg_offset_sib_base(struct insn *insn, struct pt_regs *regs); > > int insn_get_reg_offset_sib_base(struct insn *insn, struct pt_regs *regs); > > +unsigned long insn_get_seg_base(struct pt_regs *regs, struct insn *insn, > > + int regoff, bool use_default_seg); > > > > #endif /* _ASM_X86_INSN_EVAL_H */ > > diff --git a/arch/x86/lib/insn-eval.c b/arch/x86/lib/insn-eval.c > > index 8608adf..383ca83 100644 > > --- a/arch/x86/lib/insn-eval.c > > +++ b/arch/x86/lib/insn-eval.c > > @@ -355,6 +355,72 @@ static int get_desc(unsigned short seg, struct > > desc_struct **desc) > > } > > > > /** > > + * insn_get_seg_base() - Obtain base address contained in descriptor > > + * @regs: Set of registers containing the segment selector > > + * @insn: Instruction structure with selector override prefixes > > + * @regoff:Operand offset, in pt_regs, of which the selector is > > needed > > + * @use_default_seg: Use the default segment instead of prefix overrides > > I'm wondering whether you really need that bool or you can deduce this > from pt_regs... I guess I'll see... > > > + * > > + * Obtain the base address of the segment descriptor as indicated by either > > + * any segment override prefixes contained in insn or the default segment > > + * applicable to the register indicated by regoff. regoff is specified as > > the > > + * offset in bytes from the base of pt_regs. > > + * > > + * Return: In protected mode, base address of the segment. It may be zero > > in > > + * certain cases for 64-bit builds and/or 64-bit applications. In > > virtual-8086 > > + * mode, the segment selector shifed 4 positions to the right. -1L in case > > of > > s/shifed/shifted/ I will correct the typo. > > > + * error. > > + */ > > +unsigned long insn_get_seg_base(struct pt_regs *regs, struct insn *insn, > > + int regoff, bool use_default_seg) > > +{ > > + struct desc_struct *desc; > > + unsigned short seg; > > + enum segment seg_type; > > + int ret; > > + > > + seg_type = resolve_seg_selector(insn, regoff, use_default_seg); > > <--- error handling. I will add it. > > And that's not really a "seg_type" but simply the "sel"-ector. I will update the variable names to reflect the fact that they are segment selectors. > And that > "enum segment" is not really a segment but an segment override prefixes > enum. Can we please get the nomenclature right first? 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 f
Re: [v6 PATCH 08/21] x86/insn-eval: Add utility function to get segment descriptor base address
On Thu, 2017-04-20 at 10:25 +0200, Borislav Petkov wrote: > > + * insn_get_seg_base() - Obtain base address contained in > descriptor > > + * @regs:Set of registers containing the segment selector > > + * @insn:Instruction structure with selector override prefixes > > + * @regoff: Operand offset, in pt_regs, of which the selector is > needed > > + * @use_default_seg: Use the default segment instead of prefix > overrides > > I'm wondering whether you really need that bool or you can deduce this > from pt_regs... I guess I'll see... 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. 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. Thanks and BR, Ricardo
Re: [v6 PATCH 08/21] x86/insn-eval: Add utility function to get segment descriptor base address
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) --
Re: [v6 PATCH 08/21] x86/insn-eval: Add utility function to get segment descriptor base address
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) --