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=-10.9 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH, MAILING_LIST_MULTI,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 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 83668C433ED for ; Tue, 4 May 2021 19:05:31 +0000 (UTC) Received: from desiato.infradead.org (desiato.infradead.org [90.155.92.199]) (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 1A7C2610E9 for ; Tue, 4 May 2021 19:05:31 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 1A7C2610E9 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=linux.microsoft.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+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=desiato.20200630; h=Sender:Content-Transfer-Encoding :Content-Type:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:Date:Message-ID:From: References:Cc:To:Subject:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=Y0CIHSr4Kayn3ckwyLCCQ/+rTjnaLawg/PACQOl8Zdk=; b=B16s8pf2cnYUrtTsNVkD0umDb 6kQDZkW+k4Vu3XH2+TvBtxfVjIA2oEUPN07lGQDCYjM54pP0uMEcKCGp6at+PyVi2ixjtam0cb3wT MAZSoE2CNylM0R4KyJUykzQ4gEGyrdBphHMqbZ4ni63WJwvOs83p5donqMuSckMMQ9EHfIKib8tIy rTB1kJN31nb3EIt+njZ0BgAG1AknJlWLYjd59L/Qt24srjXB38XyofohY7KTwwDNTbXnKCMS1xM8b Gwxw+XMm8R5fpdweUsO6F/f3XVkWJ9Te2iSwU3RWJRFe6XQggDRK6imYHsP6RkkG2cupZUPi2R+Vz NVRlhTl4A==; Received: from localhost ([::1] helo=desiato.infradead.org) by desiato.infradead.org with esmtp (Exim 4.94 #2 (Red Hat Linux)) id 1le0KQ-00Gqak-89; Tue, 04 May 2021 19:03:30 +0000 Received: from bombadil.infradead.org ([2607:7c80:54:e::133]) by desiato.infradead.org with esmtps (Exim 4.94 #2 (Red Hat Linux)) id 1le0KG-00GqaQ-0K for linux-arm-kernel@desiato.infradead.org; Tue, 04 May 2021 19:03:23 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=bombadil.20210309; h=Content-Transfer-Encoding: Content-Type:In-Reply-To:MIME-Version:Date:Message-ID:From:References:Cc:To: Subject:Sender:Reply-To:Content-ID:Content-Description; bh=G1MDDuSShGgHKhquMWPybcW/dZj2nj7krvY28tTDDAg=; b=MMbGv7VZ8yqOY2Q09385Lp0Trk A41TnrxZ8rp0ooFnO3J9Tn7kKmJBTu+UTXdh3oIVOpWdjNUvHSCJ4AXgYAxRDo8ln7qHHVf7k0vwH KfSmi6n/2XYWWoO3g/weHlt/xB77NiHdM3zRwdf6yjhE3U5sW0kh6FSP9J/j3ftbi+lNlS9lsLKBc vtSchophR3lq8RkH4wjkNbhZc7q6PMBmeeneAftmiRtqKLEZXJcp62pCMfF2Puf4xS12UaIDzRkUb oW38SdiwhY+D1dOfgbMZcnpTkBqtWpVKHWeq+SeRzZN6VUgX2PPj/JoGDjOS+dsCbKQSS3f2FPQVr 8xG/MazQ==; Received: from linux.microsoft.com ([13.77.154.182]) by bombadil.infradead.org with esmtp (Exim 4.94 #2 (Red Hat Linux)) id 1le0KD-004CL4-3W for linux-arm-kernel@lists.infradead.org; Tue, 04 May 2021 19:03:18 +0000 Received: from [192.168.254.32] (unknown [47.187.223.33]) by linux.microsoft.com (Postfix) with ESMTPSA id 9A8B320B7178; Tue, 4 May 2021 12:03:15 -0700 (PDT) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com 9A8B320B7178 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.microsoft.com; s=default; t=1620154996; bh=G1MDDuSShGgHKhquMWPybcW/dZj2nj7krvY28tTDDAg=; h=Subject:To:Cc:References:From:Date:In-Reply-To:From; b=YEM0YiG3Y6RfRByGkVUn4qkAjaF38zJ5ZbXjCkm/aUUvaeQfn2Oy2a/IX3c/9+rJU 9W/yO/tfIV3HJhSWtpp01oKqwxF0yvgkLRNu/GhogI8DSw0u45YcpHrcRBuc1rBnrC bsyM2fAVSxntr+UItts08y0jn9IKLRyt6Vyznsuc= Subject: Re: [RFC PATCH v3 2/4] arm64: Check the return PC against unreliable code sections To: Mark Brown Cc: jpoimboe@redhat.com, mark.rutland@arm.com, jthierry@redhat.com, catalin.marinas@arm.com, will@kernel.org, jmorris@namei.org, pasha.tatashin@soleen.com, linux-arm-kernel@lists.infradead.org, live-patching@vger.kernel.org, linux-kernel@vger.kernel.org References: <65cf4dfbc439b010b50a0c46ec500432acde86d6> <20210503173615.21576-1-madvenka@linux.microsoft.com> <20210503173615.21576-3-madvenka@linux.microsoft.com> <20210504160508.GC7094@sirena.org.uk> From: "Madhavan T. Venkataraman" Message-ID: <1bd2b177-509a-21d9-e349-9b2388db45eb@linux.microsoft.com> Date: Tue, 4 May 2021 14:03:14 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.7.1 MIME-Version: 1.0 In-Reply-To: <20210504160508.GC7094@sirena.org.uk> Content-Language: en-US X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210504_120317_235067_A6FBD17C X-CRM114-Status: GOOD ( 28.39 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On 5/4/21 11:05 AM, Mark Brown wrote: > On Mon, May 03, 2021 at 12:36:13PM -0500, madvenka@linux.microsoft.com wrote: >> From: "Madhavan T. Venkataraman" >> >> Create a sym_code_ranges[] array to cover the following text sections that >> contain functions defined as SYM_CODE_*(). These functions are low-level > > This makes sense to me - a few of bikesheddy comments below but nothing > really substantive. > OK. >> +static struct code_range *lookup_range(unsigned long pc) > > This feels like it should have a prefix on the name (eg, unwinder_) > since it looks collision prone. Or lookup_code_range() rather than just > plain lookup_range(). > I will add the prefix. >> +{ > + struct code_range *range; > + > + for (range = sym_code_ranges; range->start; range++) { > > It seems more idiomatic to use ARRAY_SIZE() rather than a sentinel here, > the array can't be empty. > If there is a match, I return the matched range. Else, I return the sentinel. This is just so I don't have to check for range == NULL after calling lookup_range(). I will change it to what you have suggested and check for NULL explicitly. It is not a problem. >> + range = lookup_range(frame->pc); >> + >> #ifdef CONFIG_FUNCTION_GRAPH_TRACER >> if (tsk->ret_stack && >> frame->pc == (unsigned long)return_to_handler) { >> @@ -118,9 +160,21 @@ int notrace unwind_frame(struct task_struct *tsk, struct stackframe *frame) >> return -EINVAL; >> frame->pc = ret_stack->ret; >> frame->pc = ptrauth_strip_insn_pac(frame->pc); >> + return 0; >> } > > Do we not need to look up the range of the restored pc and validate > what's being pointed to here? It's not immediately obvious why we do > the lookup before handling the function graph tracer, especially given > that we never look at the result and there's now a return added skipping > further reliability checks. At the very least I think this needs some > additional comments so the code is more obvious. I want sym_code_ranges[] to contain both unwindable and non-unwindable ranges. Unwindable ranges will be special ranges such as the return_to_handler() and kretprobe_trampoline() functions for which the unwinder has (or will have) special code to unwind. So, the lookup_range() has to happen before the function graph code. Please look at the last patch in the series for the fix for the above function graph code. On the question of "should the original return address be checked against sym_code_ranges[]?" - I assumed that if there is a function graph trace on a function, it had to be an ftraceable function. It would not be a part of sym_code_ranges[]. Is that a wrong assumption on my part? Madhavan _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel