From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932199AbcHKRj1 (ORCPT ); Thu, 11 Aug 2016 13:39:27 -0400 Received: from mx1.redhat.com ([209.132.183.28]:38952 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750998AbcHKRjY (ORCPT ); Thu, 11 Aug 2016 13:39:24 -0400 Date: Thu, 11 Aug 2016 19:39:21 +0200 From: Oleg Nesterov To: Denys Vlasenko Cc: Ingo Molnar , Jim Keniston , Masami Hiramatsu , Srikar Dronamraju , x86@kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] uprobes/x86: fix rip-relative handling of EVEX-encoded insns. Message-ID: <20160811173921.GB31803@redhat.com> References: <20160811154521.20469-1-dvlasenk@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20160811154521.20469-1-dvlasenk@redhat.com> User-Agent: Mutt/1.5.24 (2015-08-30) X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.28]); Thu, 11 Aug 2016 17:39:24 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 > CC: Jim Keniston > CC: Masami Hiramatsu > CC: Srikar Dronamraju > CC: Ingo Molnar > CC: Oleg Nesterov > CC: x86@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 >