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 Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id B80D9C433EF for ; Wed, 27 Apr 2022 17:32:16 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S244279AbiD0RfY (ORCPT ); Wed, 27 Apr 2022 13:35:24 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:55186 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S244246AbiD0Re7 (ORCPT ); Wed, 27 Apr 2022 13:34:59 -0400 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by lindbergh.monkeyblade.net (Postfix) with ESMTP id 08FD51FC0D5 for ; Wed, 27 Apr 2022 10:31:45 -0700 (PDT) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id BDAF7143D; Wed, 27 Apr 2022 10:31:44 -0700 (PDT) Received: from lakrids.cambridge.arm.com (usa-sjc-imap-foss1.foss.arm.com [10.121.207.14]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPA id 89A483F73B; Wed, 27 Apr 2022 10:31:43 -0700 (PDT) From: Mark Rutland To: linux-arm-kernel@lists.infradead.org Cc: akpm@linux-foundation.org, alex.popov@linux.com, catalin.marinas@arm.com, keescook@chromium.org, linux-kernel@vger.kernel.org, luto@kernel.org, mark.rutland@arm.com, will@kernel.org Subject: [PATCH v2 05/13] stackleak: clarify variable names Date: Wed, 27 Apr 2022 18:31:20 +0100 Message-Id: <20220427173128.2603085-6-mark.rutland@arm.com> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20220427173128.2603085-1-mark.rutland@arm.com> References: <20220427173128.2603085-1-mark.rutland@arm.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org The logic within __stackleak_erase() can be a little hard to follow, as `boundary` switches from being the low bound to the high bound mid way through the function, and `kstack_ptr` is used to represent the start of the region to erase while `boundary` represents the end of the region to erase. Make this a little clearer by consistently using clearer variable names. The `boundary` variable is removed, the bounds of the region to erase are described by `erase_low` and `erase_high`, and bounds of the task stack are described by `task_stack_low` and `task_stck_high`. As the same time, remove the comment above the variables, since it is unclear whether it's intended as rationale, a complaint, or a TODO, and is more confusing than helpful. There should be no functional change as a result of this patch. Signed-off-by: Mark Rutland Cc: Alexander Popov Cc: Andrew Morton Cc: Andy Lutomirski Cc: Kees Cook --- kernel/stackleak.c | 30 ++++++++++++++---------------- 1 file changed, 14 insertions(+), 16 deletions(-) diff --git a/kernel/stackleak.c b/kernel/stackleak.c index 24b7cf01b2972..d5f684dc0a2d9 100644 --- a/kernel/stackleak.c +++ b/kernel/stackleak.c @@ -73,40 +73,38 @@ late_initcall(stackleak_sysctls_init); static __always_inline void __stackleak_erase(void) { const unsigned long task_stack_low = stackleak_task_low_bound(current); - - /* It would be nice not to have 'kstack_ptr' and 'boundary' on stack */ - unsigned long kstack_ptr = current->lowest_stack; - unsigned long boundary = task_stack_low; + unsigned long erase_low = current->lowest_stack; + unsigned long erase_high; unsigned int poison_count = 0; const unsigned int depth = STACKLEAK_SEARCH_DEPTH / sizeof(unsigned long); /* Search for the poison value in the kernel stack */ - while (kstack_ptr > boundary && poison_count <= depth) { - if (*(unsigned long *)kstack_ptr == STACKLEAK_POISON) + while (erase_low > task_stack_low && poison_count <= depth) { + if (*(unsigned long *)erase_low == STACKLEAK_POISON) poison_count++; else poison_count = 0; - kstack_ptr -= sizeof(unsigned long); + erase_low -= sizeof(unsigned long); } #ifdef CONFIG_STACKLEAK_METRICS - current->prev_lowest_stack = kstack_ptr; + current->prev_lowest_stack = erase_low; #endif /* - * Now write the poison value to the kernel stack. Start from - * 'kstack_ptr' and move up till the new 'boundary'. We assume that - * the stack pointer doesn't change when we write poison. + * Now write the poison value to the kernel stack between 'erase_low' + * and 'erase_high'. We assume that the stack pointer doesn't change + * when we write poison. */ if (on_thread_stack()) - boundary = current_stack_pointer; + erase_high = current_stack_pointer; else - boundary = current_top_of_stack(); + erase_high = current_top_of_stack(); - while (kstack_ptr < boundary) { - *(unsigned long *)kstack_ptr = STACKLEAK_POISON; - kstack_ptr += sizeof(unsigned long); + while (erase_low < erase_high) { + *(unsigned long *)erase_low = STACKLEAK_POISON; + erase_low += sizeof(unsigned long); } /* Reset the 'lowest_stack' value for the next syscall */ -- 2.30.2 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 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 smtp.lore.kernel.org (Postfix) with ESMTPS id D1CCFC433F5 for ; Wed, 27 Apr 2022 17:43:43 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:References:In-Reply-To: Message-Id:Date:Subject:Cc:To:From:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=0RBlK5HaKKTzHWMbAdM8rd3Jo64Cb4J8uVCUGLp4fn8=; b=M4lNzI+fPknEtv Eos9dx5NO7KghJuvkXPiwbiaDLpf//AzMExNirwBnTsxHTGS0Lc8Ynz3e7qmqCy/ziSarOH9BMPZ+ BO6SigO6d0yPsBJYS1W9Pc4gpRmichTaOZwJvR7e6oDN4b1nRmGylUwPjPjx39aoxa4coo8nZzq61 bwgWqMMZSqTy08kLACw9Q3wmRvDMUf64bW+v5r/4/uBPxoxBSMy5yWlj+tmgy4xSEtwZCANbe7XGQ 8TxxeJrEeU4ap3kLYjanPiHSObVnLhMrgfMsuDTLWHOABXpOC8wLuf5VDwOl1OEXS1lAbb/m8AY0o SZvLdbWJBQAFYGa5UkoA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1njlgT-002kNT-DI; Wed, 27 Apr 2022 17:42:37 +0000 Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1njlVx-002gY7-Tv for linux-arm-kernel@lists.infradead.org; Wed, 27 Apr 2022 17:31:47 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id BDAF7143D; Wed, 27 Apr 2022 10:31:44 -0700 (PDT) Received: from lakrids.cambridge.arm.com (usa-sjc-imap-foss1.foss.arm.com [10.121.207.14]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPA id 89A483F73B; Wed, 27 Apr 2022 10:31:43 -0700 (PDT) From: Mark Rutland To: linux-arm-kernel@lists.infradead.org Cc: akpm@linux-foundation.org, alex.popov@linux.com, catalin.marinas@arm.com, keescook@chromium.org, linux-kernel@vger.kernel.org, luto@kernel.org, mark.rutland@arm.com, will@kernel.org Subject: [PATCH v2 05/13] stackleak: clarify variable names Date: Wed, 27 Apr 2022 18:31:20 +0100 Message-Id: <20220427173128.2603085-6-mark.rutland@arm.com> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20220427173128.2603085-1-mark.rutland@arm.com> References: <20220427173128.2603085-1-mark.rutland@arm.com> MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220427_103146_113160_2C1530D6 X-CRM114-Status: GOOD ( 16.68 ) 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 The logic within __stackleak_erase() can be a little hard to follow, as `boundary` switches from being the low bound to the high bound mid way through the function, and `kstack_ptr` is used to represent the start of the region to erase while `boundary` represents the end of the region to erase. Make this a little clearer by consistently using clearer variable names. The `boundary` variable is removed, the bounds of the region to erase are described by `erase_low` and `erase_high`, and bounds of the task stack are described by `task_stack_low` and `task_stck_high`. As the same time, remove the comment above the variables, since it is unclear whether it's intended as rationale, a complaint, or a TODO, and is more confusing than helpful. There should be no functional change as a result of this patch. Signed-off-by: Mark Rutland Cc: Alexander Popov Cc: Andrew Morton Cc: Andy Lutomirski Cc: Kees Cook --- kernel/stackleak.c | 30 ++++++++++++++---------------- 1 file changed, 14 insertions(+), 16 deletions(-) diff --git a/kernel/stackleak.c b/kernel/stackleak.c index 24b7cf01b2972..d5f684dc0a2d9 100644 --- a/kernel/stackleak.c +++ b/kernel/stackleak.c @@ -73,40 +73,38 @@ late_initcall(stackleak_sysctls_init); static __always_inline void __stackleak_erase(void) { const unsigned long task_stack_low = stackleak_task_low_bound(current); - - /* It would be nice not to have 'kstack_ptr' and 'boundary' on stack */ - unsigned long kstack_ptr = current->lowest_stack; - unsigned long boundary = task_stack_low; + unsigned long erase_low = current->lowest_stack; + unsigned long erase_high; unsigned int poison_count = 0; const unsigned int depth = STACKLEAK_SEARCH_DEPTH / sizeof(unsigned long); /* Search for the poison value in the kernel stack */ - while (kstack_ptr > boundary && poison_count <= depth) { - if (*(unsigned long *)kstack_ptr == STACKLEAK_POISON) + while (erase_low > task_stack_low && poison_count <= depth) { + if (*(unsigned long *)erase_low == STACKLEAK_POISON) poison_count++; else poison_count = 0; - kstack_ptr -= sizeof(unsigned long); + erase_low -= sizeof(unsigned long); } #ifdef CONFIG_STACKLEAK_METRICS - current->prev_lowest_stack = kstack_ptr; + current->prev_lowest_stack = erase_low; #endif /* - * Now write the poison value to the kernel stack. Start from - * 'kstack_ptr' and move up till the new 'boundary'. We assume that - * the stack pointer doesn't change when we write poison. + * Now write the poison value to the kernel stack between 'erase_low' + * and 'erase_high'. We assume that the stack pointer doesn't change + * when we write poison. */ if (on_thread_stack()) - boundary = current_stack_pointer; + erase_high = current_stack_pointer; else - boundary = current_top_of_stack(); + erase_high = current_top_of_stack(); - while (kstack_ptr < boundary) { - *(unsigned long *)kstack_ptr = STACKLEAK_POISON; - kstack_ptr += sizeof(unsigned long); + while (erase_low < erase_high) { + *(unsigned long *)erase_low = STACKLEAK_POISON; + erase_low += sizeof(unsigned long); } /* Reset the 'lowest_stack' value for the next syscall */ -- 2.30.2 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel