From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752209AbdGLWMs (ORCPT ); Wed, 12 Jul 2017 18:12:48 -0400 Received: from mx1.redhat.com ([209.132.183.28]:36488 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751032AbdGLWMq (ORCPT ); Wed, 12 Jul 2017 18:12:46 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 9CAA2110E Authentication-Results: ext-mx05.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx05.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=jpoimboe@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com 9CAA2110E Date: Wed, 12 Jul 2017 17:12:42 -0500 From: Josh Poimboeuf To: Matthias Kaehlcke Cc: Chris J Arges , Borislav Petkov , Thomas Gleixner , Ingo Molnar , "H . Peter Anvin" , x86@kernel.org, linux-kernel@vger.kernel.org, Douglas Anderson , Michael Davidson , Greg Hackmann , Nick Desaulniers , Stephen Hines , Kees Cook , Arnd Bergmann , Bernhard.Rosenkranzer@linaro.org Subject: Re: [PATCH] Revert "x86/uaccess: Add stack frame output operand in get_user() inline asm" Message-ID: <20170712221242.puv5illztsla4nph@treble> References: <20170712212744.23660-1-mka@chromium.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20170712212744.23660-1-mka@chromium.org> User-Agent: Mutt/1.6.0.1 (2016-04-01) X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.29]); Wed, 12 Jul 2017 22:12:46 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Jul 12, 2017 at 02:27:44PM -0700, Matthias Kaehlcke wrote: > Commit f05058c4d652 supposedly "forces a stack frame to be created before > the inline asm code if CONFIG_FRAME_POINTER is enabled by listing the > stack pointer as an output operand for the get_user() inline assembly > statement.". This doesn't work as intended, at least with gcc v4.9.2 and > x86-64 the generated code is exactly the same with and without the patch. > However clang adds an extra instruction that adjusts %rsp, which ends up > causing double faults all over the place. I don't think reverting it is the right approach, because that will still break frame pointers in certain cases. The original commit probably should have clarified: " ... forces a stack frame *if it doesn't already exist*." In *most* cases it will have no effect, as you saw, because users of get_user() tend to do other function calls beforehand, so they will have already saved the frame pointer before calling it. However, that isn't always the case. We found that certain configs change GCC's behavior such that, for certain get_user() call sites, the containing function doesn't saved the frame pointer before inserting get_user()'s inline asm. GCC completely ignores inline asm, so it has no idea that it has a call instruction in it. So in general, *any* inline asm with a call instruction needs this constraint, to force the frame pointer to be saved, if it hasn't already. This is admittedly an awkward way of achieving this goal, but it's the only way I know how to do it with GCC. What extra instruction does clang add? -- Josh