Well, I guess I have to reply...
So let me say that I am not going to even try to review this change. I know
absolutely nothing about x86 insn decoding, but I tend to trust Denys in
this area.
On 08/11, Denys Vlasenko wrote:
>
> Since instruction decoder now supports EVEX-encoded insns, two fixes are
> needed
> to correctly handle them in uprobes.
>
> Extended bits for MODRM.rm field need to be sanitized just like we do it
> for VEX3, to avoid encoding wrong register for register-relative access.
> EVEX has _two_ extended bits: b and x. Theoretically, EVEX.x should be
> ignored by CPU (since GPRs go only up to 15, not 31), but let's be paranoid
> here: proper encoding for register-relative access should have EVEX.x = 1.
>
> Secondly, we should fetch vex.vvvv for EVEX too.
> This is now super easy because insn decoder populates vex_prefix.bytes[2]
> for all flavors of (e)vex encodings, even for vex2.
>
> Signed-off-by: Denys Vlasenko <dvlas...@redhat.com>
> CC: Jim Keniston <jkeni...@us.ibm.com>
> CC: Masami Hiramatsu <masami.hiramatsu...@hitachi.com>
> CC: Srikar Dronamraju <sri...@linux.vnet.ibm.com>
> CC: Ingo Molnar <mi...@kernel.org>
> CC: Oleg Nesterov <o...@redhat.com>
> CC: x...@kernel.org
> CC: linux-kernel@vger.kernel.org
> ---
> arch/x86/kernel/uprobes.c | 22 +++++++++++-----------
> 1 file changed, 11 insertions(+), 11 deletions(-)
>
> diff --git a/arch/x86/kernel/uprobes.c b/arch/x86/kernel/uprobes.c
> index 6c1ff31..a6e6096 100644
> --- a/arch/x86/kernel/uprobes.c
> +++ b/arch/x86/kernel/uprobes.c
> @@ -357,20 +357,22 @@ static void riprel_analyze(struct arch_uprobe *auprobe,
> struct insn *insn)
> *cursor &= 0xfe;
> }
> /*
> - * Similar treatment for VEX3 prefix.
> - * TODO: add XOP/EVEX treatment when insn decoder supports them
> + * Similar treatment for VEX3/EVEX prefix.
> + * TODO: add XOP treatment when insn decoder supports them
> */
> - if (insn->vex_prefix.nbytes == 3) {
> + if (insn->vex_prefix.nbytes >= 3) {
> /*
> * vex2: c5 rvvvvLpp (has no b bit)
> * vex3/xop: c4/8f rxbmmmmm wvvvvLpp
> * evex: 62 rxbR00mm wvvvv1pp zllBVaaa
> - * (evex will need setting of both b and x since
> - * in non-sib encoding evex.x is 4th bit of MODRM.rm)
> - * Setting VEX3.b (setting because it has inverted meaning):
> + * Setting VEX3.b (setting because it has inverted meaning).
> + * Setting EVEX.x since (in non-SIB encoding) EVEX.x
> + * is the 4th bit of MODRM.rm, and needs the same treatment.
> + * For VEX3-encoded insns, VEX3.x value has no effect in
> + * non-SIB encoding, the change is superfluous but harmless.
> */
> cursor = auprobe->insn + insn_offset_vex_prefix(insn) + 1;
> - *cursor |= 0x20;
> + *cursor |= 0x60;
> }
>
> /*
> @@ -415,12 +417,10 @@ static void riprel_analyze(struct arch_uprobe *auprobe,
> struct insn *insn)
>
> reg = MODRM_REG(insn); /* Fetch modrm.reg */
> reg2 = 0xff; /* Fetch vex.vvvv */
> - if (insn->vex_prefix.nbytes == 2)
> - reg2 = insn->vex_prefix.bytes[1];
> - else if (insn->vex_prefix.nbytes == 3)
> + if (insn->vex_prefix.nbytes)
> reg2 = insn->vex_prefix.bytes[2];
> /*
> - * TODO: add XOP, EXEV vvvv reading.
> + * TODO: add XOP vvvv reading.
> *
> * vex.vvvv field is in bits 6-3, bits are inverted.
> * But in 32-bit mode, high-order bit may be ignored.
> --
> 2.9.2
>
