Re: [PATCH v7 13/26] x86/insn-eval: Add function to get default params of code segment
On Wed, 2017-06-07 at 14:59 +0200, Borislav Petkov wrote: > On Fri, May 05, 2017 at 11:17:11AM -0700, Ricardo Neri wrote: > > This function returns the default values of the address and operand sizes > > as specified in the segment descriptor. This information is determined > > from the D and L bits. Hence, it can be used for both IA-32e 64-bit and > > 32-bit legacy modes. For virtual-8086 mode, the default address and > > operand sizes are always 2 bytes. > > > > The D bit is only meaningful for code segments. Thus, these functions > > always use the code segment selector contained in regs. > > > > 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 | 6 > > arch/x86/lib/insn-eval.c | 65 > > > > 2 files changed, 71 insertions(+) > > > > diff --git a/arch/x86/include/asm/insn-eval.h > > b/arch/x86/include/asm/insn-eval.h > > index 7f3c7fe..9ed1c88 100644 > > --- a/arch/x86/include/asm/insn-eval.h > > +++ b/arch/x86/include/asm/insn-eval.h > > @@ -11,9 +11,15 @@ > > #include > > #include > > > > +struct insn_code_seg_defaults { > > A whole struct for a function which gets called only once? > > Bah, that's a bit too much, if you ask me. > > So you're returning two small unsigned integers - i.e., you can just as > well return a single u8 and put address and operand sizes in there: > > ret = oper_sz | addr_sz << 4; > > No need for special structs for that. OK. This makes sense. Perhaps I can use a couple of #define's to set and get the the address and operand sizes in a single u8. This would make the code more readable. > > > + unsigned char address_bytes; > > + unsigned char operand_bytes; > > +}; > > + > > void __user *insn_get_addr_ref(struct insn *insn, struct pt_regs *regs); > > int insn_get_modrm_rm_off(struct insn *insn, struct pt_regs *regs); > > unsigned long insn_get_seg_base(struct pt_regs *regs, struct insn *insn, > > int regoff); > > +struct insn_code_seg_defaults insn_get_code_seg_defaults(struct pt_regs > > *regs); > > > > #endif /* _ASM_X86_INSN_EVAL_H */ > > diff --git a/arch/x86/lib/insn-eval.c b/arch/x86/lib/insn-eval.c > > index c77ed80..693e5a8 100644 > > --- a/arch/x86/lib/insn-eval.c > > +++ b/arch/x86/lib/insn-eval.c > > @@ -603,6 +603,71 @@ static unsigned long get_seg_limit(struct pt_regs > > *regs, struct insn *insn, > > } > > > > /** > > + * insn_get_code_seg_defaults() - Obtain code segment default parameters > > + * @regs: Structure with register values as seen when entering kernel mode > > + * > > + * Obtain the default parameters of the code segment: address and operand > > sizes. > > + * The code segment is obtained from the selector contained in the CS > > register > > + * in regs. In protected mode, the default address is determined by > > inspecting > > + * the L and D bits of the segment descriptor. In virtual-8086 mode, the > > default > > + * is always two bytes for both address and operand sizes. > > + * > > + * Return: A populated insn_code_seg_defaults structure on success. The > > + * structure contains only zeros on failure. > > s/failure/error/ Will correct. > > > + */ > > +struct insn_code_seg_defaults insn_get_code_seg_defaults(struct pt_regs > > *regs) > > +{ > > + struct desc_struct *desc; > > + struct insn_code_seg_defaults defs; > > + unsigned short sel; > > + /* > > +* The most significant byte of AR_TYPE_MASK determines whether a > > +* segment contains data or code. > > +*/ > > + unsigned int type_mask = AR_TYPE_MASK & (1 << 11); > > + > > + memset(, 0, sizeof(defs)); > > + > > + if (v8086_mode(regs)) { > > + defs.address_bytes = 2; > > + defs.operand_bytes = 2; > > + return defs; > > + } > > + > > + sel = (unsigned short)regs->cs; > > + > > + desc = get_desc(sel); > > + if (!desc) > > + return defs; > > + > > + /* if data segment, return */ > > + if (!(desc->b & type_mask)) > > + return defs; > > So you can simplify that into: > > /* A code segment? */ > if (!(desc->b & BIT(11))) > return defs; > > and remove that type_mask thing. Alternatively, I can do desc->type & BIT(3) to avoid
Re: [PATCH v7 13/26] x86/insn-eval: Add function to get default params of code segment
On Fri, May 05, 2017 at 11:17:11AM -0700, Ricardo Neri wrote: > This function returns the default values of the address and operand sizes > as specified in the segment descriptor. This information is determined > from the D and L bits. Hence, it can be used for both IA-32e 64-bit and > 32-bit legacy modes. For virtual-8086 mode, the default address and > operand sizes are always 2 bytes. > > The D bit is only meaningful for code segments. Thus, these functions > always use the code segment selector contained in regs. > > 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 | 6 > arch/x86/lib/insn-eval.c | 65 > > 2 files changed, 71 insertions(+) > > diff --git a/arch/x86/include/asm/insn-eval.h > b/arch/x86/include/asm/insn-eval.h > index 7f3c7fe..9ed1c88 100644 > --- a/arch/x86/include/asm/insn-eval.h > +++ b/arch/x86/include/asm/insn-eval.h > @@ -11,9 +11,15 @@ > #include > #include > > +struct insn_code_seg_defaults { A whole struct for a function which gets called only once? Bah, that's a bit too much, if you ask me. So you're returning two small unsigned integers - i.e., you can just as well return a single u8 and put address and operand sizes in there: ret = oper_sz | addr_sz << 4; No need for special structs for that. > + unsigned char address_bytes; > + unsigned char operand_bytes; > +}; > + > void __user *insn_get_addr_ref(struct insn *insn, struct pt_regs *regs); > int insn_get_modrm_rm_off(struct insn *insn, struct pt_regs *regs); > unsigned long insn_get_seg_base(struct pt_regs *regs, struct insn *insn, > int regoff); > +struct insn_code_seg_defaults insn_get_code_seg_defaults(struct pt_regs > *regs); > > #endif /* _ASM_X86_INSN_EVAL_H */ > diff --git a/arch/x86/lib/insn-eval.c b/arch/x86/lib/insn-eval.c > index c77ed80..693e5a8 100644 > --- a/arch/x86/lib/insn-eval.c > +++ b/arch/x86/lib/insn-eval.c > @@ -603,6 +603,71 @@ static unsigned long get_seg_limit(struct pt_regs *regs, > struct insn *insn, > } > > /** > + * insn_get_code_seg_defaults() - Obtain code segment default parameters > + * @regs:Structure with register values as seen when entering kernel mode > + * > + * Obtain the default parameters of the code segment: address and operand > sizes. > + * The code segment is obtained from the selector contained in the CS > register > + * in regs. In protected mode, the default address is determined by > inspecting > + * the L and D bits of the segment descriptor. In virtual-8086 mode, the > default > + * is always two bytes for both address and operand sizes. > + * > + * Return: A populated insn_code_seg_defaults structure on success. The > + * structure contains only zeros on failure. s/failure/error/ > + */ > +struct insn_code_seg_defaults insn_get_code_seg_defaults(struct pt_regs > *regs) > +{ > + struct desc_struct *desc; > + struct insn_code_seg_defaults defs; > + unsigned short sel; > + /* > + * The most significant byte of AR_TYPE_MASK determines whether a > + * segment contains data or code. > + */ > + unsigned int type_mask = AR_TYPE_MASK & (1 << 11); > + > + memset(, 0, sizeof(defs)); > + > + if (v8086_mode(regs)) { > + defs.address_bytes = 2; > + defs.operand_bytes = 2; > + return defs; > + } > + > + sel = (unsigned short)regs->cs; > + > + desc = get_desc(sel); > + if (!desc) > + return defs; > + > + /* if data segment, return */ > + if (!(desc->b & type_mask)) > + return defs; So you can simplify that into: /* A code segment? */ if (!(desc->b & BIT(11))) return defs; and remove that type_mask thing. -- Regards/Gruss, Boris. SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg) -- -- 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
[PATCH v7 13/26] x86/insn-eval: Add function to get default params of code segment
This function returns the default values of the address and operand sizes as specified in the segment descriptor. This information is determined from the D and L bits. Hence, it can be used for both IA-32e 64-bit and 32-bit legacy modes. For virtual-8086 mode, the default address and operand sizes are always 2 bytes. The D bit is only meaningful for code segments. Thus, these functions always use the code segment selector contained in regs. Cc: Dave HansenCc: 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 | 6 arch/x86/lib/insn-eval.c | 65 2 files changed, 71 insertions(+) diff --git a/arch/x86/include/asm/insn-eval.h b/arch/x86/include/asm/insn-eval.h index 7f3c7fe..9ed1c88 100644 --- a/arch/x86/include/asm/insn-eval.h +++ b/arch/x86/include/asm/insn-eval.h @@ -11,9 +11,15 @@ #include #include +struct insn_code_seg_defaults { + unsigned char address_bytes; + unsigned char operand_bytes; +}; + void __user *insn_get_addr_ref(struct insn *insn, struct pt_regs *regs); int insn_get_modrm_rm_off(struct insn *insn, struct pt_regs *regs); unsigned long insn_get_seg_base(struct pt_regs *regs, struct insn *insn, int regoff); +struct insn_code_seg_defaults insn_get_code_seg_defaults(struct pt_regs *regs); #endif /* _ASM_X86_INSN_EVAL_H */ diff --git a/arch/x86/lib/insn-eval.c b/arch/x86/lib/insn-eval.c index c77ed80..693e5a8 100644 --- a/arch/x86/lib/insn-eval.c +++ b/arch/x86/lib/insn-eval.c @@ -603,6 +603,71 @@ static unsigned long get_seg_limit(struct pt_regs *regs, struct insn *insn, } /** + * insn_get_code_seg_defaults() - Obtain code segment default parameters + * @regs: Structure with register values as seen when entering kernel mode + * + * Obtain the default parameters of the code segment: address and operand sizes. + * The code segment is obtained from the selector contained in the CS register + * in regs. In protected mode, the default address is determined by inspecting + * the L and D bits of the segment descriptor. In virtual-8086 mode, the default + * is always two bytes for both address and operand sizes. + * + * Return: A populated insn_code_seg_defaults structure on success. The + * structure contains only zeros on failure. + */ +struct insn_code_seg_defaults insn_get_code_seg_defaults(struct pt_regs *regs) +{ + struct desc_struct *desc; + struct insn_code_seg_defaults defs; + unsigned short sel; + /* +* The most significant byte of AR_TYPE_MASK determines whether a +* segment contains data or code. +*/ + unsigned int type_mask = AR_TYPE_MASK & (1 << 11); + + memset(, 0, sizeof(defs)); + + if (v8086_mode(regs)) { + defs.address_bytes = 2; + defs.operand_bytes = 2; + return defs; + } + + sel = (unsigned short)regs->cs; + + desc = get_desc(sel); + if (!desc) + return defs; + + /* if data segment, return */ + if (!(desc->b & type_mask)) + return defs; + + switch ((desc->l << 1) | desc->d) { + case 0: /* Legacy mode. CS.L=0, CS.D=0 */ + defs.address_bytes = 2; + defs.operand_bytes = 2; + break; + case 1: /* Legacy mode. CS.L=0, CS.D=1 */ + defs.address_bytes = 4; + defs.operand_bytes = 4; + break; + case 2: /* IA-32e 64-bit mode. CS.L=1, CS.D=0 */ + defs.address_bytes = 8; + defs.operand_bytes = 4; + break; + case 3: /* Invalid setting. CS.L=1, CS.D=1 */ + /* fall through */ + default: + defs.address_bytes = 0; + defs.operand_bytes = 0; + } + + return defs; +} + +/** * insn_get_reg_offset_modrm_rm() - Obtain register in r/m part of ModRM byte * @insn: Instruction structure containing the ModRM byte * @regs: Structure with register values as seen when entering kernel mode -- 2.9.3 -- 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