From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751428AbdFGPsk (ORCPT ); Wed, 7 Jun 2017 11:48:40 -0400 Received: from mx2.suse.de ([195.135.220.15]:37398 "EHLO mx1.suse.de" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1750831AbdFGPsi (ORCPT ); Wed, 7 Jun 2017 11:48:38 -0400 Date: Wed, 7 Jun 2017 17:48:19 +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 , 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 , Arnaldo Carvalho de Melo , Adrian Hunter , Kees Cook , Thomas Garnier , Dmitry Vyukov Subject: Re: [PATCH v7 16/26] x86/insn-eval: Support both signed 32-bit and 64-bit effective addresses Message-ID: <20170607154819.xkbxp3hg7lwjdxd6@pd.tnic> References: <20170505181724.55000-1-ricardo.neri-calderon@linux.intel.com> <20170505181724.55000-17-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: <20170505181724.55000-17-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 Fri, May 05, 2017 at 11:17:14AM -0700, Ricardo Neri wrote: > The 32-bit and 64-bit address encodings are identical. This means that we > can use the same function in both cases. In order to reuse the function > for 32-bit address encodings, we must sign-extend our 32-bit signed > operands to 64-bit signed variables (only for 64-bit builds). To decide on > whether sign extension is needed, we rely on the address size as given by > the instruction structure. > > Once the effective address has been computed, a special verification is > needed for 32-bit processes. If running on a 64-bit kernel, such processes > can address up to 4GB of memory. Hence, for instance, an effective > address of 0xffff1234 would be misinterpreted as 0xffffffffffff1234 due to > the sign extension mentioned above. For this reason, the 4 must be Which 4? > truncated to obtain the true effective address. > > Lastly, before computing the linear address, we verify that the effective > address is within the limits of the segment. The check is kept for long > mode because in such a case the limit is set to -1L. This is the largest > unsigned number possible. This is equivalent to a limit-less segment. > > 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: x86@kernel.org > Signed-off-by: Ricardo Neri > --- > arch/x86/lib/insn-eval.c | 99 ++++++++++++++++++++++++++++++++++++++++++------ > 1 file changed, 88 insertions(+), 11 deletions(-) > > diff --git a/arch/x86/lib/insn-eval.c b/arch/x86/lib/insn-eval.c > index 1a5f5a6..c7c1239 100644 > --- a/arch/x86/lib/insn-eval.c > +++ b/arch/x86/lib/insn-eval.c > @@ -688,6 +688,62 @@ int insn_get_modrm_rm_off(struct insn *insn, struct pt_regs *regs) > return get_reg_offset(insn, regs, REG_TYPE_RM); > } > > +/** > + * _to_signed_long() - Cast an unsigned long into signed long > + * @val A 32-bit or 64-bit unsigned long > + * @long_bytes The number of bytes used to represent a long number > + * @out The casted signed long > + * > + * Return: A signed long of either 32 or 64 bits, as per the build configuration > + * of the kernel. > + */ > +static int _to_signed_long(unsigned long val, int long_bytes, long *out) > +{ > + if (!out) > + return -EINVAL; > + > +#ifdef CONFIG_X86_64 > + if (long_bytes == 4) { > + /* higher bytes should all be zero */ > + if (val & ~0xffffffff) > + return -EINVAL; > + > + /* sign-extend to a 64-bit long */ So this is a 32-bit userspace on a 64-bit kernel, right? If so, how can a memory offset be > 32-bits and we have to extend it to a 64-bit long?!? I *think* you want to say that you want to convert it to long so that you can do the calculation in longs. However! If you're a 64-bit kernel running a 32-bit userspace, you need to do the calculation in 32-bits only so that it overflows, as it would do on 32-bit hardware. IOW, the clamping to 32-bits at the end is not something you wanna do but actually let it wrap if it overflows. Or am I missing something? -- Regards/Gruss, Boris. SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg) --