Messages in this thread Patch in this message | | | Date | Thu, 20 Apr 2017 10:25:33 +0200 | From | Borislav Petkov <> | Subject | 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 <dave.hansen@linux.intel.com> > Cc: Adam Buchbinder <adam.buchbinder@gmail.com> > Cc: Colin Ian King <colin.king@canonical.com> > Cc: Lorenzo Stoakes <lstoakes@gmail.com> > Cc: Qiaowei Ren <qiaowei.ren@intel.com> > Cc: Arnaldo Carvalho de Melo <acme@redhat.com> > Cc: Masami Hiramatsu <mhiramat@kernel.org> > Cc: Adrian Hunter <adrian.hunter@intel.com> > Cc: Kees Cook <keescook@chromium.org> > Cc: Thomas Garnier <thgarnie@google.com> > Cc: Peter Zijlstra <peterz@infradead.org> > Cc: Borislav Petkov <bp@suse.de> > Cc: Dmitry Vyukov <dvyukov@google.com> > Cc: Ravi V. Shankar <ravi.v.shankar@intel.com> > Cc: x86@kernel.org > Signed-off-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com> > --- > 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. > + */ > + 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 byte > * @regs: Set of registers indicated by the ModRM byte > -- > 2.9.3 >
-- Regards/Gruss, Boris.
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg) --
| |