From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751722AbdG1CEy (ORCPT ); Thu, 27 Jul 2017 22:04:54 -0400 Received: from mga11.intel.com ([192.55.52.93]:32320 "EHLO mga11.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751607AbdG1CEx (ORCPT ); Thu, 27 Jul 2017 22:04:53 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.40,423,1496127600"; d="scan'208";a="132918693" Message-ID: <1501207492.22603.68.camel@ranerica-desktop> Subject: Re: [PATCH v7 16/26] x86/insn-eval: Support both signed 32-bit and 64-bit effective addresses From: Ricardo Neri To: Borislav Petkov 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 Date: Thu, 27 Jul 2017 19:04:52 -0700 In-Reply-To: <20170727132635.GA28553@nazgul.tnic> References: <20170505181724.55000-1-ricardo.neri-calderon@linux.intel.com> <20170505181724.55000-17-ricardo.neri-calderon@linux.intel.com> <20170607154819.xkbxp3hg7lwjdxd6@pd.tnic> <1501026493.22603.48.camel@ranerica-desktop> <20170727132635.GA28553@nazgul.tnic> Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.10.4-0ubuntu2 Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, 2017-07-27 at 15:26 +0200, Borislav Petkov wrote: > On Tue, Jul 25, 2017 at 04:48:13PM -0700, Ricardo Neri wrote: > > I meant to say the 4 most significant bytes. In this case, the > > 64-address 0xffffffffffff1234 would lie in the kernel memory while > > 0xffff1234 would correctly be in the user space memory. > > That explanation is better. > > > Yes, perhaps the check above is not needed. I included that check as > > part of my argument validation. In a 64-bit kernel, this function could > > be called with val with non-zero most significant bytes. > > So say that in the comment so that it is obvious *why*. > > > I have looked into this closely and as far as I can see, the 4 least > > significant bytes will wrap around when using 64-bit signed numbers as > > they would when using 32-bit signed numbers. For instance, for two > > positive numbers we have: > > > > 7fff:ffff + 7000:0000 = efff:ffff. > > > > The addition above overflows. > > Yes, MSB changes. > > > When sign-extended to 64-bit numbers we would have: > > > > 0000:0000:7fff:ffff + 0000:0000:7000:0000 = 0000:0000:efff:ffff. > > > > The addition above does not overflow. However, the 4 least significant > > bytes overflow as we expect. > > No they don't - you are simply using 64-bit regs: > > 0x00005555555546b8 <+8>: movq $0x7fffffff,-0x8(%rbp) > 0x00005555555546c0 <+16>: movq $0x70000000,-0x10(%rbp) > 0x00005555555546c8 <+24>: mov -0x8(%rbp),%rdx > 0x00005555555546cc <+28>: mov -0x10(%rbp),%rax > => 0x00005555555546d0 <+32>: add %rdx,%rax > > rax 0xefffffff 4026531839 > rbx 0x0 0 > rcx 0x0 0 > rdx 0x7fffffff 2147483647 > > ... > > eflags 0x206 [ PF IF ] > > (OF flag is not set). True, I don't have the OF set. However the 4 least significant bytes wrapped around; which is what I needed. > > > We can clamp the 4 most significant bytes. > > > > For a two's complement negative numbers we can have: > > > > ffff:ffff + 8000:0000 = 7fff:ffff with a carry flag. > > > > The addition above overflows. > > Yes. > > > When sign-extending to 64-bit numbers we would have: > > > > ffff:ffff:ffff:ffff + ffff:ffff:8000:0000 = ffff:ffff:7fff:ffff with a > > carry flag. > > > > The addition above does not overflow. However, the 4 least significant > > bytes overflew and wrapped around as they would when using 32-bit signed > > numbers. > > Right. Ok. > > And come to think of it now, I'm wondering, whether it would be > better/easier/simpler/more straight-forward, to do the 32-bit operations > with 32-bit types and separate 32-bit functions and have the hardware do > that for you. > > This way you can save yourself all that ugly and possibly error-prone > casting back and forth and have the code much more readable too. That sounds fair. I had to explain a lot this code and probably is not worth it. I can definitely use 32-bit variable types for the 32-bit case and drop all these castings. The 32-bit and 64-bit functions would look identical except for the variables used to compute the effective address. Perhaps I could use a union: union eff_addr { #if CONFIG_X86_64 long addr64; #endif int addr32; }; And use one or the other based on the address size given by the CS.L CS.D bits of the segment descriptor or address size overrides. However using the union could be less readable than having two almost identical functions. Thanks and BR, Ricardo