From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753917AbdDKLbx (ORCPT ); Tue, 11 Apr 2017 07:31:53 -0400 Received: from mx2.suse.de ([195.135.220.15]:36235 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752291AbdDKLbs (ORCPT ); Tue, 11 Apr 2017 07:31:48 -0400 Date: Tue, 11 Apr 2017 13:31:26 +0200 From: Borislav Petkov To: Ricardo Neri Cc: Ingo Molnar , Thomas Gleixner , "H. Peter Anvin" , Andy Lutomirski , Peter Zijlstra , Andrew Morton , Brian Gerst , Chris Metcalf , Dave Hansen , Paolo Bonzini , Liang Z Li , Masami Hiramatsu , Huang Rui , Jiri Slaby , Jonathan Corbet , "Michael S. Tsirkin" , Paul Gortmaker , Vlastimil Babka , Chen Yucong , Alexandre Julliard , Stas Sergeev , Fenghua Yu , "Ravi V. Shankar" , Shuah Khan , linux-kernel@vger.kernel.org, x86@kernel.org, linux-msdos@vger.kernel.org, wine-devel@winehq.org, Adam Buchbinder , Colin Ian King , Lorenzo Stoakes , Qiaowei Ren , Nathan Howard , Adan Hawthorn , Joe Perches Subject: Re: [v6 PATCH 02/21] x86/mpx: Do not use SIB index if index points to R/ESP Message-ID: <20170411113126.ygq52fdoe6rp6jzw@pd.tnic> References: <20170308003254.27833-1-ricardo.neri-calderon@linux.intel.com> <20170308003254.27833-3-ricardo.neri-calderon@linux.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20170308003254.27833-3-ricardo.neri-calderon@linux.intel.com> User-Agent: NeoMutt/20170113 (1.7.2) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Mar 07, 2017 at 04:32:35PM -0800, 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 memory addressing is used > (i.e., mod part of ModR/M is not 3), a SIB byte is used and the index of > the SIB byte points to the R/ESP (i.e., index = 4), the index should not be > used in the computation of the memory address. > > In these cases the address is simply the value present in the register > pointed by the base part of the SIB byte plus the displacement byte. > > An example of such instruction could be > > insn -0x80(%rsp) > > This is represented as: > > [opcode] 4c 23 80 > > ModR/M=0x4c: mod: 0x1, reg: 0x1: r/m: 0x4(R/ESP) > SIB=0x23: sc: 0, index: 0x100(R/ESP), base: 0x11(R/EBX): > Displacement -0x80 > > The correct address is (base) + displacement; no index is used. > > We can achieve the desired effect of not using the index by making > get_reg_offset return -EDOM in this particular case. This value indicates > callers that they should not use the index to calculate the address. > EINVAL continues to indicate that an error when decoding the SIB byte. > > Care is taken to allow R12 to be used as index, which is a valid scenario. > > 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: x86@kernel.org > Signed-off-by: Ricardo Neri > --- > arch/x86/mm/mpx.c | 19 +++++++++++++++++-- > 1 file changed, 17 insertions(+), 2 deletions(-) > > diff --git a/arch/x86/mm/mpx.c b/arch/x86/mm/mpx.c > index ff112e3..d9e92d6 100644 > --- a/arch/x86/mm/mpx.c > +++ b/arch/x86/mm/mpx.c > @@ -110,6 +110,13 @@ static int get_reg_offset(struct insn *insn, struct pt_regs *regs, > regno = X86_SIB_INDEX(insn->sib.value); > if (X86_REX_X(insn->rex_prefix.value)) > regno += 8; > + /* > + * If mod !=3, register R/ESP (regno=4) is not used as index in > + * the address computation. Check is done after looking at REX.X > + * This is because R12 (regno=12) can be used as an index. > + */ > + if (regno == 4 && X86_MODRM_MOD(insn->modrm.value) != 3) > + return -EDOM; Hmm, ok, so this is a bit confusing, to me at least. Maybe you're saying the same things but here's how I see it: 1. When ModRM.mod != 11b and ModRM.rm == 100b, all that does mean is that you have a SIB byte following. I.e., you have indexed register-indirect addressing. Now, you still need to decode the SIB byte and it goes this way: SIB.index == 100b means that the index register specification is null, i.e., the scale*index portion of that indexed register-indirect addressing is null, i.e., you have an offset following the SIB byte. Now, depending on ModRM.mod, that offset is: ModRM.mod == 01b -> 1 byte offset ModRM.mod == 10b -> 4 bytes offset That's why for an instruction like this one (let's use your example) you have: 8b 4c 23 80 mov -0x80(%rbx,%riz,1),%ecx That's basically a binutils hack to state that the SIB index register is null. Another SIB index register works, of course: 8b 4c 03 80 mov -0x80(%rbx,%rax,1),%ecx Ok, so far so good. 2. Now, the %r12 thing is part of the REX implications to those encodings: That's the REX.X bit which adds a fourth bit to the encoding of the SIB base register, i.e., if you specify a register with SIB.index, you want to be able to specify all 16 regs, thus the 4th bit. That's why it says that the SIB byte is required for %r12-based addressing. I.e., you can still have a SIB.index == 100b addressing with an index register which is not null but that is only because SIB.index is now {REX.X=1b, 100b}, i.e.: Prefixes: REX: 0x43 { 4 [w]: 0 [r]: 0 [x]: 1 [b]: 1 } Opcode: 0x8b ModRM: 0x4c [mod:1b][.R:0b,reg:1b][.B:1b,r/m:1100b] register-indirect mode, 1-byte offset in displ. field SIB: 0x63 [.B:1b,base:1011b][.X:1b,idx:1100b][scale: 1] MOV Gv,Ev; MOV reg{16,32,64} reg/mem{16,32,64} 0: 43 8b 4c 63 80 mov -0x80(%r11,%r12,2),%ecx So, I'm not saying your version is necessarily wrong - I'm just saying that it could explain the situation a bit more verbose. Btw, I'd flip the if-test above: if (X86_MODRM_MOD(insn->modrm.value) != 3 && regno == 4) to make it just like the order the conditions are specified in the manuals. Thanks. -- Regards/Gruss, Boris. SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg) --