Re: [PATCH v7 09/26] x86/insn-eval: Add utility function to identify string instructions
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 operands of string instructions with two operands. Btw, LODS and OUTS use only DS:rSI as a source operand. So we have to be careful with the generalization here. So if ES:rDI is the only seg. reg we want, then we don't need to look at those insns... (we assume DS by default). ... > +/** > + * is_string_instruction - Determine if instruction is a string instruction > + * @insn:Instruction structure containing the opcode > + * > + * Return: true if the instruction, determined by the opcode, is any of the > + * string instructions as defined in the Intel Software Development manual. > + * False otherwise. > + */ > +static bool is_string_instruction(struct insn *insn) > +{ > + insn_get_opcode(insn); > + > + /* all string instructions have a 1-byte opcode */ > + if (insn->opcode.nbytes != 1) > + return false; > + > + switch (insn->opcode.bytes[0]) { > + case INSB: > + /* fall through */ > + case INSW_INSD: > + /* fall through */ > + case OUTSB: > + /* fall through */ > + case OUTSW_OUTSD: > + /* fall through */ > + case MOVSB: > + /* fall through */ > + case MOVSW_MOVSD: > + /* fall through */ > + case CMPSB: > + /* fall through */ > + case CMPSW_CMPSD: > + /* fall through */ > + case STOSB: > + /* fall through */ > + case STOSW_STOSD: > + /* fall through */ > + case LODSB: > + /* fall through */ > + case LODSW_LODSD: > + /* fall through */ > + case SCASB: > + /* fall through */ That "fall through" for every opcode is just too much. Also, you can use the regularity of the x86 opcode space and do: case 0x6c ... 0x6f: /* INS/OUTS */ case 0xa4 ... 0xa7: /* MOVS/CMPS */ case 0xaa ... 0xaf: /* STOS/LODS/SCAS */ return true; default: return false; } And voila, there's your compact is_string_insn() function! :^) (Modulo the exact list, as I mentioned above). Thanks. -- 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
Re: [PATCH v7 08/26] x86/insn-eval: Add a utility function to get register offsets
On Fri, May 05, 2017 at 11:17:06AM -0700, Ricardo Neri wrote: > The function get_reg_offset() returns the offset to the register the > argument specifies as indicated in an enumeration of type offset. Callers > of this function would need the definition of such enumeration. This is > not needed. Instead, add helper functions for this purpose. These functions > are useful in cases when, for instance, the caller needs to decide whether > the operand is a register or a memory location by looking at the rm part > of the ModRM byte. As of now, this is the only helper function that is > needed. > > 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 | 1 + > arch/x86/lib/insn-eval.c | 15 +++ > 2 files changed, 16 insertions(+) > > diff --git a/arch/x86/include/asm/insn-eval.h > b/arch/x86/include/asm/insn-eval.h > index 5cab1b1..7e8c963 100644 > --- a/arch/x86/include/asm/insn-eval.h > +++ b/arch/x86/include/asm/insn-eval.h > @@ -12,5 +12,6 @@ > #include > > 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); > > #endif /* _ASM_X86_INSN_EVAL_H */ > diff --git a/arch/x86/lib/insn-eval.c b/arch/x86/lib/insn-eval.c > index 182e2ae..8b16761 100644 > --- a/arch/x86/lib/insn-eval.c > +++ b/arch/x86/lib/insn-eval.c > @@ -97,6 +97,21 @@ static int get_reg_offset(struct insn *insn, struct > pt_regs *regs, > return regoff[regno]; > } > > +/** > + * insn_get_reg_offset_modrm_rm() - Obtain register in r/m part of ModRM byte That name needs to be synced with the function name below. > + * @insn:Instruction structure containing the ModRM byte > + * @regs:Structure with register values as seen when entering kernel mode > + * > + * Return: The register indicated by the r/m part of the ModRM byte. The > + * register is obtained as an offset from the base of pt_regs. In specific > + * cases, the returned value can be -EDOM to indicate that the particular > value > + * of ModRM does not refer to a register and shall be ignored. > + */ > +int insn_get_modrm_rm_off(struct insn *insn, struct pt_regs *regs) -- 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
Re: [PATCH v7 07/26] x86/insn-eval: Do not BUG on invalid register type
On Fri, May 05, 2017 at 11:17:05AM -0700, Ricardo Neri wrote: > We are not in a critical failure path. The invalid register type is caused > when trying to decode invalid instruction bytes from a user-space program. > Thus, simply print an error message. To prevent this warning from being > abused from user space programs, use the rate-limited variant of printk. > > Cc: Borislav Petkov> Cc: Andy Lutomirski > 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: Dmitry Vyukov > Cc: Ravi V. Shankar > Cc: x...@kernel.org > Signed-off-by: Ricardo Neri > --- > arch/x86/lib/insn-eval.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/arch/x86/lib/insn-eval.c b/arch/x86/lib/insn-eval.c > index e746a6f..182e2ae 100644 > --- a/arch/x86/lib/insn-eval.c > +++ b/arch/x86/lib/insn-eval.c > @@ -5,6 +5,7 @@ > */ > #include > #include > +#include > #include > #include > #include > @@ -85,9 +86,8 @@ static int get_reg_offset(struct insn *insn, struct pt_regs > *regs, > break; > > default: > - pr_err("invalid register type"); > - BUG(); > - break; > + printk_ratelimited(KERN_ERR "insn-eval: x86: invalid register > type"); You can use pr_err_ratelimited() and define "insn-eval" with pr_fmt. Look for examples in the tree. Btw, "insn-eval" is perhaps not the right name - since we're building an instruction decoder, maybe it should be called "insn-dec" or so. I'm looking at those other arch/x86/lib/insn.c, arch/x86/include/asm/inat.h things and how they're starting to morph into one decoding facility, AFAICT. -- 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
_3дрaвствyйтe! Вaс uнтeрeсyют клueнтскue бaзы дaнныx?
_Зqравствyйте! Bас интересyют kлиентсkие 6азы qанных?
Re: [PATCH v7 05/26] x86/mpx: Do not use SIB.base if its value is 101b and ModRM.mod = 0
On Fri, May 05, 2017 at 11:17:03AM -0700, 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 is base = 101b and the mod part > of the ModRM byte is zero, the base port on the effective address > computation is null. In this case, a 32-bit displacement follows the SIB > byte. This is obtained when the instruction decoder parses the operands. > > To signal this scenario, a -EDOM error is returned to indicate callers that > they should ignore the base. > > Cc: Borislav Petkov> Cc: Andy Lutomirski > Cc: Dave Hansen > Cc: Adam Buchbinder > Cc: Colin Ian King > Cc: Lorenzo Stoakes > Cc: Qiaowei Ren > Cc: Peter Zijlstra > Cc: Nathan Howard > Cc: Adan Hawthorn > Cc: Joe Perches > Cc: Ravi V. Shankar > Cc: x...@kernel.org > Signed-off-by: Ricardo Neri > --- > arch/x86/mm/mpx.c | 27 --- > 1 file changed, 20 insertions(+), 7 deletions(-) > > diff --git a/arch/x86/mm/mpx.c b/arch/x86/mm/mpx.c > index 7397b81..30aef92 100644 > --- a/arch/x86/mm/mpx.c > +++ b/arch/x86/mm/mpx.c > @@ -122,6 +122,15 @@ static int get_reg_offset(struct insn *insn, struct > pt_regs *regs, > > case REG_TYPE_BASE: > regno = X86_SIB_BASE(insn->sib.value); > + /* > + * If ModRM.mod is 0 and SIB.base == 5, the base of the > + * register-indirect addressing is 0. In this case, a > + * 32-bit displacement is expected in this case; the > + * instruction decoder finds such displacement for us. That last sentence reads funny. Just say: "In this case, a 32-bit displacement follows the SIB byte." > + */ > + if (!X86_MODRM_MOD(insn->modrm.value) && regno == 5) > + return -EDOM; > + > if (X86_REX_B(insn->rex_prefix.value)) > regno += 8; > break; -- Regards/Gruss, Boris. Good mailing practices for 400: avoid top-posting and trim the reply. -- 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