On Wed, 2017-05-31 at 18:58 +0200, Borislav Petkov wrote:
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 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.
...
diff --git a/arch/x86/lib/insn-eval.c b/arch/x86/lib/insn-eval.c index f46cb31..c77ed80 100644 --- a/arch/x86/lib/insn-eval.c +++ b/arch/x86/lib/insn-eval.c @@ -476,6 +476,133 @@ static struct desc_struct *get_desc(unsigned short sel) }
/**
- insn_get_seg_base() - Obtain base address of segment descriptor.
- @regs: Structure with register values as seen when entering kernel mode
- @insn: Instruction structure with selector override prefixes
- @regoff: Operand offset, in pt_regs, of which the selector is needed
- 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. Zero in for long
- mode, except when FS or GS are used. In virtual-8086 mode, the segment
- selector shifted 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)+{
- struct desc_struct *desc;
- unsigned short sel;
- enum segment_register seg_reg;
- seg_reg = resolve_seg_register(insn, regs, regoff);
- if (seg_reg == SEG_REG_INVAL)
return -1L;- sel = get_segment_selector(regs, seg_reg);
- if ((short)sel < 0)
I guess it would be better if that function returned a signed short so you don't have to cast it here. (You're casting it to an unsigned long below anyway.)
Yes, this make sense. I will make this change.
return -1L;- if (v8086_mode(regs))
/** Base is simply the segment selector shifted 4* positions to the right.*/return (unsigned long)(sel << 4);...
+static unsigned long get_seg_limit(struct pt_regs *regs, struct insn *insn,
int regoff)+{
- struct desc_struct *desc;
- unsigned short sel;
- unsigned long limit;
- enum segment_register seg_reg;
- seg_reg = resolve_seg_register(insn, regs, regoff);
- if (seg_reg == SEG_REG_INVAL)
return 0;- sel = get_segment_selector(regs, seg_reg);
- if ((short)sel < 0)
Ditto.
Here as well.
return 0;- if (user_64bit_mode(regs) || v8086_mode(regs))
return -1L;- if (!sel)
return 0;- desc = get_desc(sel);
- if (!desc)
return 0;- /*
* If the granularity bit is set, the limit is given in multiples* of 4096. When the granularity bit is set, the least 12 significantthe 12 least significant bits
* bits are not tested when checking the segment limits. In practice,* this means that the segment ends in (limit << 12) + 0xfff.*/- limit = get_desc_limit(desc);
- if (desc->g)
limit <<= 12 | 0x7;That 0x7 doesn't look like 0xfff - it shifts limit by 15 instead. You can simply write it like you mean it:
limit = (limit << 12) + 0xfff;
You are right, this wrong. I will implement as you mention.
Thanks and BR, Ricardo