From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.5 required=3.0 tests=DKIM_INVALID,DKIM_SIGNED, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 9127CC352BE for ; Fri, 17 Apr 2020 10:01:23 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 720FC21D91 for ; Fri, 17 Apr 2020 10:01:23 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (2048-bit key) header.d=infradead.org header.i=@infradead.org header.b="G25zyJyL" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728113AbgDQKBW (ORCPT ); Fri, 17 Apr 2020 06:01:22 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:50040 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726207AbgDQKBV (ORCPT ); Fri, 17 Apr 2020 06:01:21 -0400 Received: from bombadil.infradead.org (bombadil.infradead.org [IPv6:2607:7c80:54:e::133]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id C6E52C061A0C for ; Fri, 17 Apr 2020 03:01:21 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=bombadil.20170209; h=In-Reply-To:Content-Type:MIME-Version :References:Message-ID:Subject:Cc:To:From:Date:Sender:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description; bh=/sCrLJsk9M0RFgQwNkauJ/Dtc5/K6rCJg0npReZJ/ZM=; b=G25zyJyLZs4ijof9NKQ93s/RtD tp1CABmZSWsvURGWx1WS3YUul4JFfkwKfPlMdOUMLdf0WJVJTQmYH7kEnhXsJQOx0kyS3h3w+4g5E s+xd6331OtUvGqwz0rttWZMMrZR+P8JR5WVKgcxW9SWh1X+w7ZXxIONjudPt4TumeaAQPaOiHvfJf hqJFmA4o3d/ICe7K81YdhfKX/Cu65J5gdmf91MuW2YmlJLqzGWkPWtZYiYRHVPbv0K/7xLORS8GtG SAJt6bXYa7kxdX69qaFDx4rwpwhNl0i+c4WbpER2GL+jX2fzS/+iDQNUr+t6P36U7llVMhvrYMKxh yu548iOQ==; Received: from j217100.upc-j.chello.nl ([24.132.217.100] helo=noisy.programming.kicks-ass.net) by bombadil.infradead.org with esmtpsa (Exim 4.92.3 #3 (Red Hat Linux)) id 1jPNne-000501-Rg; Fri, 17 Apr 2020 10:00:43 +0000 Received: from hirez.programming.kicks-ass.net (hirez.programming.kicks-ass.net [192.168.1.225]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by noisy.programming.kicks-ass.net (Postfix) with ESMTPS id 816003010BC; Fri, 17 Apr 2020 12:00:39 +0200 (CEST) Received: by hirez.programming.kicks-ass.net (Postfix, from userid 1000) id 6AB682B120750; Fri, 17 Apr 2020 12:00:39 +0200 (CEST) Date: Fri, 17 Apr 2020 12:00:39 +0200 From: Peter Zijlstra To: Sami Tolvanen Cc: Will Deacon , Catalin Marinas , James Morse , Steven Rostedt , Ard Biesheuvel , Mark Rutland , Masahiro Yamada , Michal Marek , Ingo Molnar , Juri Lelli , Vincent Guittot , Dave Martin , Kees Cook , Laura Abbott , Marc Zyngier , Masami Hiramatsu , Nick Desaulniers , Jann Horn , Miguel Ojeda , clang-built-linux@googlegroups.com, kernel-hardening@lists.openwall.com, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v11 04/12] scs: disable when function graph tracing is enabled Message-ID: <20200417100039.GS20730@hirez.programming.kicks-ass.net> References: <20191018161033.261971-1-samitolvanen@google.com> <20200416161245.148813-1-samitolvanen@google.com> <20200416161245.148813-5-samitolvanen@google.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20200416161245.148813-5-samitolvanen@google.com> Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Apr 16, 2020 at 09:12:37AM -0700, Sami Tolvanen wrote: > The graph tracer hooks returns by modifying frame records on the > (regular) stack, but with SCS the return address is taken from the > shadow stack, and the value in the frame record has no effect. As we > don't currently have a mechanism to determine the corresponding slot > on the shadow stack (and to pass this through the ftrace > infrastructure), for now let's disable SCS when the graph tracer is > enabled. > > With SCS the return address is taken from the shadow stack and the > value in the frame record has no effect. The mcount based graph tracer > hooks returns by modifying frame records on the (regular) stack, and > thus is not compatible. The patchable-function-entry graph tracer > used for DYNAMIC_FTRACE_WITH_REGS modifies the LR before it is saved > to the shadow stack, and is compatible. > > Modifying the mcount based graph tracer to work with SCS would require > a mechanism to determine the corresponding slot on the shadow stack > (and to pass this through the ftrace infrastructure), and we expect > that everyone will eventually move to the patchable-function-entry > based graph tracer anyway, so for now let's disable SCS when the > mcount-based graph tracer is enabled. > > SCS and patchable-function-entry are both supported from LLVM 10.x. SCS would actually provide another way to do return hooking. An arguably much saner model at that. The 'normal' way is to (temporary) replace the on-stack return value, and then replace it back in the return handler. This is because we can't simply push a fake return on the stack, because that would wreck the expected stack layout of the regular function. But there is nothing that would stop us from pushing an extra entry on the SCS. It would in fact be a much cleaner solution. The entry hook sticks an extra entry on the SCS, the function ignores what's on the normal stack and pops from the SCS, we return to the exit handler, which in turn pops from the SCS stack at which point we're back to regular. The only 'funny' is that the exit handler itself should not push to the SCS, or we should frob the return-to-exit-handler such that it lands after the push. > Signed-off-by: Sami Tolvanen > Reviewed-by: Kees Cook > Reviewed-by: Mark Rutland > --- > arch/Kconfig | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/arch/Kconfig b/arch/Kconfig > index 691a552c2cc3..c53cb9025ad2 100644 > --- a/arch/Kconfig > +++ b/arch/Kconfig > @@ -542,6 +542,7 @@ config ARCH_SUPPORTS_SHADOW_CALL_STACK > > config SHADOW_CALL_STACK > bool "Clang Shadow Call Stack" > + depends on DYNAMIC_FTRACE_WITH_REGS || !FUNCTION_GRAPH_TRACER > depends on ARCH_SUPPORTS_SHADOW_CALL_STACK > help > This option enables Clang's Shadow Call Stack, which uses a AFAICT you also need to kill KRETPROBES, which plays similar games. And doesn't BPF also do stuff like this? From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.8 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI, SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 574BDC352BE for ; Fri, 17 Apr 2020 10:00:47 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 28E6A21D91 for ; Fri, 17 Apr 2020 10:00:47 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="azegVPA/" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 28E6A21D91 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=infradead.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20170209; h=Sender: Content-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=oB9Pd2Z67rWgDcRoTwRySxSb1uoYJMUqEI97a9JsXng=; b=azegVPA/KyKrzi XyeB0tJZZ/1feJqsPLDZ7N9AyUNfg8Pgdgm1xAEt3BL81SbSnEHyeYiQw/HM3uj8oMbbQK8Xy5+wW Tcm8LTE8UFSPf9UQbeeRn2zEhn5tev6l+RQ1kBqugYb40D1V14pxPc7vhRtmWkwe81UpRDvmB68nT Kuj+OYoIy/ZRGLjuTf7PDaTJPYcVZ6juwk3ATQoYm1LEF0tG3DeK47mBgjVOFznim7uFyqVPmisNE pYiBX4ivrnB0n0NakPIKlVPHGiiedKabAiWTK32UYZLVGNyMYjif/QdoRg+fSfMLMkDUSvjv0Kwwx iuLfNb8JVStIdq2C/pDA==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1jPNni-00051G-Ji; Fri, 17 Apr 2020 10:00:46 +0000 Received: from j217100.upc-j.chello.nl ([24.132.217.100] helo=noisy.programming.kicks-ass.net) by bombadil.infradead.org with esmtpsa (Exim 4.92.3 #3 (Red Hat Linux)) id 1jPNne-000501-Rg; Fri, 17 Apr 2020 10:00:43 +0000 Received: from hirez.programming.kicks-ass.net (hirez.programming.kicks-ass.net [192.168.1.225]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by noisy.programming.kicks-ass.net (Postfix) with ESMTPS id 816003010BC; Fri, 17 Apr 2020 12:00:39 +0200 (CEST) Received: by hirez.programming.kicks-ass.net (Postfix, from userid 1000) id 6AB682B120750; Fri, 17 Apr 2020 12:00:39 +0200 (CEST) Date: Fri, 17 Apr 2020 12:00:39 +0200 From: Peter Zijlstra To: Sami Tolvanen Subject: Re: [PATCH v11 04/12] scs: disable when function graph tracing is enabled Message-ID: <20200417100039.GS20730@hirez.programming.kicks-ass.net> References: <20191018161033.261971-1-samitolvanen@google.com> <20200416161245.148813-1-samitolvanen@google.com> <20200416161245.148813-5-samitolvanen@google.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20200416161245.148813-5-samitolvanen@google.com> X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Mark Rutland , Juri Lelli , kernel-hardening@lists.openwall.com, Catalin Marinas , Will Deacon , Marc Zyngier , Masahiro Yamada , clang-built-linux@googlegroups.com, Ingo Molnar , Laura Abbott , Dave Martin , Kees Cook , Jann Horn , Steven Rostedt , linux-arm-kernel@lists.infradead.org, Michal Marek , Ard Biesheuvel , Nick Desaulniers , linux-kernel@vger.kernel.org, Miguel Ojeda , James Morse , Masami Hiramatsu Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Thu, Apr 16, 2020 at 09:12:37AM -0700, Sami Tolvanen wrote: > The graph tracer hooks returns by modifying frame records on the > (regular) stack, but with SCS the return address is taken from the > shadow stack, and the value in the frame record has no effect. As we > don't currently have a mechanism to determine the corresponding slot > on the shadow stack (and to pass this through the ftrace > infrastructure), for now let's disable SCS when the graph tracer is > enabled. > > With SCS the return address is taken from the shadow stack and the > value in the frame record has no effect. The mcount based graph tracer > hooks returns by modifying frame records on the (regular) stack, and > thus is not compatible. The patchable-function-entry graph tracer > used for DYNAMIC_FTRACE_WITH_REGS modifies the LR before it is saved > to the shadow stack, and is compatible. > > Modifying the mcount based graph tracer to work with SCS would require > a mechanism to determine the corresponding slot on the shadow stack > (and to pass this through the ftrace infrastructure), and we expect > that everyone will eventually move to the patchable-function-entry > based graph tracer anyway, so for now let's disable SCS when the > mcount-based graph tracer is enabled. > > SCS and patchable-function-entry are both supported from LLVM 10.x. SCS would actually provide another way to do return hooking. An arguably much saner model at that. The 'normal' way is to (temporary) replace the on-stack return value, and then replace it back in the return handler. This is because we can't simply push a fake return on the stack, because that would wreck the expected stack layout of the regular function. But there is nothing that would stop us from pushing an extra entry on the SCS. It would in fact be a much cleaner solution. The entry hook sticks an extra entry on the SCS, the function ignores what's on the normal stack and pops from the SCS, we return to the exit handler, which in turn pops from the SCS stack at which point we're back to regular. The only 'funny' is that the exit handler itself should not push to the SCS, or we should frob the return-to-exit-handler such that it lands after the push. > Signed-off-by: Sami Tolvanen > Reviewed-by: Kees Cook > Reviewed-by: Mark Rutland > --- > arch/Kconfig | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/arch/Kconfig b/arch/Kconfig > index 691a552c2cc3..c53cb9025ad2 100644 > --- a/arch/Kconfig > +++ b/arch/Kconfig > @@ -542,6 +542,7 @@ config ARCH_SUPPORTS_SHADOW_CALL_STACK > > config SHADOW_CALL_STACK > bool "Clang Shadow Call Stack" > + depends on DYNAMIC_FTRACE_WITH_REGS || !FUNCTION_GRAPH_TRACER > depends on ARCH_SUPPORTS_SHADOW_CALL_STACK > help > This option enables Clang's Shadow Call Stack, which uses a AFAICT you also need to kill KRETPROBES, which plays similar games. And doesn't BPF also do stuff like this? _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel