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 CA117C433EF for ; Sun, 15 May 2022 17:34:08 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S237892AbiEOReG (ORCPT ); Sun, 15 May 2022 13:34:06 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:46038 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S237966AbiEORdJ (ORCPT ); Sun, 15 May 2022 13:33:09 -0400 Received: from mail-ej1-f43.google.com (mail-ej1-f43.google.com [209.85.218.43]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id EE2D218E0F for ; Sun, 15 May 2022 10:33:06 -0700 (PDT) Received: by mail-ej1-f43.google.com with SMTP id n10so24742045ejk.5 for ; Sun, 15 May 2022 10:33:06 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:date:mime-version:user-agent:reply-to :subject:content-language:to:cc:references:from:in-reply-to :content-transfer-encoding; bh=uYhdYFCzTcyWy1nJvuBdObZ0FLG3j1yvGrt81DM3BkM=; b=FXLlK+hfMtfVIuIXqsSdvzT6qo2l8QPcCYM5nreRtC3UPCR5OziBvPGPVO2FZp8P5N nt6IN6EDzx+iMIrA8dyWz5HOhGCjF8VwRJ1J4efMTNKGYUHvmgmoNXNqOGvWynWf2AvS xv+bewmJzxD3sXcag+c+l5fzu0lSfBYSMn2OyGtWqRSxcHgwDC8dNURY1HeHtUeLKxj5 wJ7tauPIFgOmdJWpRlxSL1UchhWscgSVjzsWVg6Ex1PZep8FMSd18GvASj1E7xfBg3z0 lJSSHr15b1Ud18HnuoFCRU2e2GOCxjVfu6S0LBMRoHhZigQaCb8a5Xd0S/xjgRe2S3n7 OODw== X-Gm-Message-State: AOAM533AlazalHR95LsTGjC6vc/sChhSVsVyAVE+qYb5VyVyWGJjTfv9 NYaX6OWcwdH3UxIek/Ew/L0= X-Google-Smtp-Source: ABdhPJy2szvIRJK9d9TJb4SmoJW1cAmP+Fq+lWGgF11JslVeZRQyL4q2Cme+lRPtKkqdmLnlBftJmA== X-Received: by 2002:a17:907:162a:b0:6f4:c53b:fca7 with SMTP id hb42-20020a170907162a00b006f4c53bfca7mr12441587ejc.723.1652635984570; Sun, 15 May 2022 10:33:04 -0700 (PDT) Received: from [10.9.0.34] ([46.166.133.199]) by smtp.gmail.com with ESMTPSA id hg17-20020a1709072cd100b006fc308e76absm2833961ejc.2.2022.05.15.10.33.02 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Sun, 15 May 2022 10:33:04 -0700 (PDT) Message-ID: <02e40030-52a5-f23c-85be-be59a7d3211c@linux.com> Date: Sun, 15 May 2022 20:33:01 +0300 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.7.0 Reply-To: alex.popov@linux.com Subject: Re: [PATCH v2 07/13] stackleak: rework poison scanning Content-Language: en-US To: Mark Rutland Cc: linux-arm-kernel@lists.infradead.org, akpm@linux-foundation.org, catalin.marinas@arm.com, keescook@chromium.org, linux-kernel@vger.kernel.org, luto@kernel.org, will@kernel.org References: <20220427173128.2603085-1-mark.rutland@arm.com> <20220427173128.2603085-8-mark.rutland@arm.com> <268ea8f7-472b-f1d4-6b8b-0c8fefccc0fa@linux.com> From: Alexander Popov In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 10.05.2022 16:13, Mark Rutland wrote: > On Mon, May 09, 2022 at 04:51:35PM +0300, Alexander Popov wrote: >> Hello Mark! >> >> On 27.04.2022 20:31, Mark Rutland wrote: >>> Currently we over-estimate the region of stack which must be erased. >>> >>> To determine the region to be erased, we scan downards for a contiguous >>> block of poison values (or the low bound of the stack). There are a few >>> minor problems with this today: >>> >>> * When we find a block of poison values, we include this block within >>> the region to erase. >>> >>> As this is included within the region to erase, this causes us to >>> redundantly overwrite 'STACKLEAK_SEARCH_DEPTH' (128) bytes with >>> poison. >> >> Right, this can be improved. >> >>> * As the loop condition checks 'poison_count <= depth', it will run an >>> additional iteration after finding the contiguous block of poison, >>> decrementing 'erase_low' once more than necessary. >> >> Actually, I think the current code is correct. >> >> I'm intentionally searching one poison value more than >> STACKLEAK_SEARCH_DEPTH to avoid the corner case. See the BUILD_BUG_ON >> assertion in stackleak_track_stack() that describes it: >> >> /* >> * Having CONFIG_STACKLEAK_TRACK_MIN_SIZE larger than >> * STACKLEAK_SEARCH_DEPTH makes the poison search in >> * stackleak_erase() unreliable. Let's prevent that. >> */ >> BUILD_BUG_ON(CONFIG_STACKLEAK_TRACK_MIN_SIZE > STACKLEAK_SEARCH_DEPTH); > > I had read that, but as written that doesn't imply that it's necessary to scan > one more element than STACKLEAK_SEARCH_DEPTH, nor why. I'm more than happy to > change the logic, but I think we need a very clear explanation as to why we > need to scan the specific number of bytes we scan, and we should account for > that *within* STACKLEAK_SEARCH_DEPTH for clarity. I'll try to explain. The stackleak gcc plugin instruments the kernel code inserting the 'stackleak_track_stack()' calls for the functions with a stack frame size greater than or equal to CONFIG_STACKLEAK_TRACK_MIN_SIZE. The kernel functions with a smaller stack frame are not instrumented (the 'lowest_stack' value is not updated for them). Any kernel function may leave uninitialized data on its stack frame. The poison scanning must handle that correctly. The uninitialized groups of poison values must be smaller than the search depth, otherwise 'stackleak_erase()' is unreliable. So with this BUILD_BUG_ON I control that CONFIG_STACKLEAK_TRACK_MIN_SIZE <= STACKLEAK_SEARCH_DEPTH. To be sure and avoid mistakes in the edge cases, 'stackleak_erase()' is searching one poison value more than STACKLEAK_SEARCH_DEPTH. If you don't like this one additional poison value in the search, I would propose to change the assertion. What do you think? >>> As this is included within the region to erase, this causes us to >>> redundantly overwrite an additional unsigned long with poison. >>> >>> * As we always decrement 'erase_low' after checking an element on the >>> stack, we always include the element below this within the region to >>> erase. >>> >>> As this is included within the region to erase, this causes us to >>> redundantly overwrite an additional unsigned long with poison. >>> >>> Note that this is not a functional problem. As the loop condition >>> checks 'erase_low > task_stack_low', we'll never clobber the >>> STACK_END_MAGIC. As we always decrement 'erase_low' after this, we'll >>> never fail to erase the element immediately above the STACK_END_MAGIC. >> >> Right, I don't see any bug in the current erasing code. >> >> When I wrote the current code, I carefully checked all the corner cases. For >> example, on the first stack erasing, the STACK_END_MAGIC was not >> overwritten, but the memory next to it was erased. Same for the beginning of >> the stack: I carefully checked that no unpoisoned bytes were left on the >> thread stack. >> >>> In total, this can cause us to erase `128 + 2 * sizeof(unsigned long)` >>> bytes more than necessary, which is unfortunate. >>> >>> This patch reworks the logic to find the address immediately above the >>> poisoned region, by finding the lowest non-poisoned address. This is >>> factored into a stackleak_find_top_of_poison() helper both for clarity >>> and so that this can be shared with the LKDTM test in subsequent >>> patches. >> >> You know, I wrote stackleak_erase() in very plain C. I wanted a compiler to >> generate assembly that is very close to the original asm version. I worried >> that compilers might do weird stuff with the local variables and the stack >> pointer. >> >> So I checked stackleak for gcc versions 4.8, 5, 6, 7, 8, 9, and 10 on >> x86_64, i386 and arm64. This is my project that helped with this work: >> https://github.com/a13xp0p0v/kernel-build-containers > > I've used the kernel.org cross toolchains, as published at: > > https://mirrors.edge.kernel.org/pub/tools/crosstool/ > >> Mark, in this patch series you use many local variables and helper functions. >> Honestly, this worries me. For example, compilers can (and usually do) >> ignore the presence of the 'inline' specifier for the purpose of >> optimization. > > I've deliberately used `__always_inline` rather than regular `inline` to > prevent code being placed out-of-line. As mentioned in oether replies it has a > stronger semantic. > > Thanks, > Mark. > >> >> Thanks! >> >>> Signed-off-by: Mark Rutland >>> Cc: Alexander Popov >>> Cc: Andrew Morton >>> Cc: Andy Lutomirski >>> Cc: Kees Cook >>> --- >>> include/linux/stackleak.h | 26 ++++++++++++++++++++++++++ >>> kernel/stackleak.c | 18 ++++-------------- >>> 2 files changed, 30 insertions(+), 14 deletions(-) >>> >>> diff --git a/include/linux/stackleak.h b/include/linux/stackleak.h >>> index 467661aeb4136..c36e7a3b45e7e 100644 >>> --- a/include/linux/stackleak.h >>> +++ b/include/linux/stackleak.h >>> @@ -42,6 +42,32 @@ stackleak_task_high_bound(const struct task_struct *tsk) >>> return (unsigned long)task_pt_regs(tsk); >>> } >>> +/* >>> + * Find the address immediately above the poisoned region of the stack, where >>> + * that region falls between 'low' (inclusive) and 'high' (exclusive). >>> + */ >>> +static __always_inline unsigned long >>> +stackleak_find_top_of_poison(const unsigned long low, const unsigned long high) >>> +{ >>> + const unsigned int depth = STACKLEAK_SEARCH_DEPTH / sizeof(unsigned long); >>> + unsigned int poison_count = 0; >>> + unsigned long poison_high = high; >>> + unsigned long sp = high; >>> + >>> + while (sp > low && poison_count < depth) { >>> + sp -= sizeof(unsigned long); >>> + >>> + if (*(unsigned long *)sp == STACKLEAK_POISON) { >>> + poison_count++; >>> + } else { >>> + poison_count = 0; >>> + poison_high = sp; >>> + } >>> + } >>> + >>> + return poison_high; >>> +} >>> + >>> static inline void stackleak_task_init(struct task_struct *t) >>> { >>> t->lowest_stack = stackleak_task_low_bound(t); >>> diff --git a/kernel/stackleak.c b/kernel/stackleak.c >>> index ba346d46218f5..afd54b8e10b83 100644 >>> --- a/kernel/stackleak.c >>> +++ b/kernel/stackleak.c >>> @@ -74,20 +74,10 @@ static __always_inline void __stackleak_erase(void) >>> { >>> const unsigned long task_stack_low = stackleak_task_low_bound(current); >>> const unsigned long task_stack_high = stackleak_task_high_bound(current); >>> - 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 (erase_low > task_stack_low && poison_count <= depth) { >>> - if (*(unsigned long *)erase_low == STACKLEAK_POISON) >>> - poison_count++; >>> - else >>> - poison_count = 0; >>> - >>> - erase_low -= sizeof(unsigned long); >>> - } >>> + unsigned long erase_low, erase_high; >>> + >>> + erase_low = stackleak_find_top_of_poison(task_stack_low, >>> + current->lowest_stack); >>> #ifdef CONFIG_STACKLEAK_METRICS >>> current->prev_lowest_stack = erase_low; >> 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 78802C433F5 for ; Sun, 15 May 2022 17:34:24 +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-Type: Content-Transfer-Encoding:Reply-To:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:From:References:Cc:To: Subject:MIME-Version:Date:Message-ID:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=s9jlOfOG6egjgsrWPLGOnmgpE0Lj/buNsgx2nj9VnfA=; b=TNq37znw9ahZIy 1Dl72flkh3tAl/+AxU1grME8PckRNvmyG5j+kgyWHSOIxQiv/YuNmCH13dK2MN735NxaWRrJCqKwh b3tjTNbTPc8NvUlkZgzb8VjxkXWkF2VT3BuSB/sYg6LAjO/gKzOdl8Tt7HEFkkrXlhHB0HaegaF+g q6efGqqJKwxIXHwPHq4SBRdeJaYDfNuYRf6kBWoXKev5ozfhshH4RdnVPUEf597fhuD54xOaFCQXa kKW+E5DDGBmkCZNO9dfP2mZ6ubke5Uppp7JE8C/D8Y8YyJWSzHAK8G8Li7yUrm/SzpqR9v48a9g5e +6HNsMMPFnp5Yn0DgO2Q==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1nqI7C-004XYD-Cu; Sun, 15 May 2022 17:33:10 +0000 Received: from mail-ej1-f45.google.com ([209.85.218.45]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1nqI79-004XXI-A1 for linux-arm-kernel@lists.infradead.org; Sun, 15 May 2022 17:33:09 +0000 Received: by mail-ej1-f45.google.com with SMTP id dk23so24693912ejb.8 for ; Sun, 15 May 2022 10:33:05 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:date:mime-version:user-agent:reply-to :subject:content-language:to:cc:references:from:in-reply-to :content-transfer-encoding; bh=uYhdYFCzTcyWy1nJvuBdObZ0FLG3j1yvGrt81DM3BkM=; b=e4sx/MjdMzU5iQDDEUem/FjAIi6k4o5JqvTXsIeHWCb48tUYDRUsASclFxQ+ZZ9+Bg 0q9VZSWhCldWMMDQ0QtT3VMl/0TQ5ixmczxfz7Na8rg+CVKMHzh2xiLVDNlelldifTZT Q44qDNIIh4v002NGeNvW6ZwN3YZTELKy+FwicC36XVU/LLT529iyrxYJpulW+nxlFZo+ DFRCfhUEyLyYm0n4eW+LLRVlqXG+JbEMy1z0RTiC/l+iJyAX5hIW4NwDeqobuqHf+zoL 256SvK1jqBZbNNIvHzz6anEG0oW3jwAGrSTMaXwH9VDhe/7HjcAEvaQ09pK6ZBGw9LfJ F78w== X-Gm-Message-State: AOAM530ycwYTqlJ3/MRC4+MzLB208DYL11eRrSdp3st0Xovhc6ewYrMi 73ZvS4wBwfoXdf/tucKDez8= X-Google-Smtp-Source: ABdhPJy2szvIRJK9d9TJb4SmoJW1cAmP+Fq+lWGgF11JslVeZRQyL4q2Cme+lRPtKkqdmLnlBftJmA== X-Received: by 2002:a17:907:162a:b0:6f4:c53b:fca7 with SMTP id hb42-20020a170907162a00b006f4c53bfca7mr12441587ejc.723.1652635984570; Sun, 15 May 2022 10:33:04 -0700 (PDT) Received: from [10.9.0.34] ([46.166.133.199]) by smtp.gmail.com with ESMTPSA id hg17-20020a1709072cd100b006fc308e76absm2833961ejc.2.2022.05.15.10.33.02 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Sun, 15 May 2022 10:33:04 -0700 (PDT) Message-ID: <02e40030-52a5-f23c-85be-be59a7d3211c@linux.com> Date: Sun, 15 May 2022 20:33:01 +0300 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.7.0 Subject: Re: [PATCH v2 07/13] stackleak: rework poison scanning Content-Language: en-US To: Mark Rutland Cc: linux-arm-kernel@lists.infradead.org, akpm@linux-foundation.org, catalin.marinas@arm.com, keescook@chromium.org, linux-kernel@vger.kernel.org, luto@kernel.org, will@kernel.org References: <20220427173128.2603085-1-mark.rutland@arm.com> <20220427173128.2603085-8-mark.rutland@arm.com> <268ea8f7-472b-f1d4-6b8b-0c8fefccc0fa@linux.com> From: Alexander Popov In-Reply-To: X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220515_103307_404465_AC1DC7EA X-CRM114-Status: GOOD ( 39.07 ) 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: , Reply-To: alex.popov@linux.com Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On 10.05.2022 16:13, Mark Rutland wrote: > On Mon, May 09, 2022 at 04:51:35PM +0300, Alexander Popov wrote: >> Hello Mark! >> >> On 27.04.2022 20:31, Mark Rutland wrote: >>> Currently we over-estimate the region of stack which must be erased. >>> >>> To determine the region to be erased, we scan downards for a contiguous >>> block of poison values (or the low bound of the stack). There are a few >>> minor problems with this today: >>> >>> * When we find a block of poison values, we include this block within >>> the region to erase. >>> >>> As this is included within the region to erase, this causes us to >>> redundantly overwrite 'STACKLEAK_SEARCH_DEPTH' (128) bytes with >>> poison. >> >> Right, this can be improved. >> >>> * As the loop condition checks 'poison_count <= depth', it will run an >>> additional iteration after finding the contiguous block of poison, >>> decrementing 'erase_low' once more than necessary. >> >> Actually, I think the current code is correct. >> >> I'm intentionally searching one poison value more than >> STACKLEAK_SEARCH_DEPTH to avoid the corner case. See the BUILD_BUG_ON >> assertion in stackleak_track_stack() that describes it: >> >> /* >> * Having CONFIG_STACKLEAK_TRACK_MIN_SIZE larger than >> * STACKLEAK_SEARCH_DEPTH makes the poison search in >> * stackleak_erase() unreliable. Let's prevent that. >> */ >> BUILD_BUG_ON(CONFIG_STACKLEAK_TRACK_MIN_SIZE > STACKLEAK_SEARCH_DEPTH); > > I had read that, but as written that doesn't imply that it's necessary to scan > one more element than STACKLEAK_SEARCH_DEPTH, nor why. I'm more than happy to > change the logic, but I think we need a very clear explanation as to why we > need to scan the specific number of bytes we scan, and we should account for > that *within* STACKLEAK_SEARCH_DEPTH for clarity. I'll try to explain. The stackleak gcc plugin instruments the kernel code inserting the 'stackleak_track_stack()' calls for the functions with a stack frame size greater than or equal to CONFIG_STACKLEAK_TRACK_MIN_SIZE. The kernel functions with a smaller stack frame are not instrumented (the 'lowest_stack' value is not updated for them). Any kernel function may leave uninitialized data on its stack frame. The poison scanning must handle that correctly. The uninitialized groups of poison values must be smaller than the search depth, otherwise 'stackleak_erase()' is unreliable. So with this BUILD_BUG_ON I control that CONFIG_STACKLEAK_TRACK_MIN_SIZE <= STACKLEAK_SEARCH_DEPTH. To be sure and avoid mistakes in the edge cases, 'stackleak_erase()' is searching one poison value more than STACKLEAK_SEARCH_DEPTH. If you don't like this one additional poison value in the search, I would propose to change the assertion. What do you think? >>> As this is included within the region to erase, this causes us to >>> redundantly overwrite an additional unsigned long with poison. >>> >>> * As we always decrement 'erase_low' after checking an element on the >>> stack, we always include the element below this within the region to >>> erase. >>> >>> As this is included within the region to erase, this causes us to >>> redundantly overwrite an additional unsigned long with poison. >>> >>> Note that this is not a functional problem. As the loop condition >>> checks 'erase_low > task_stack_low', we'll never clobber the >>> STACK_END_MAGIC. As we always decrement 'erase_low' after this, we'll >>> never fail to erase the element immediately above the STACK_END_MAGIC. >> >> Right, I don't see any bug in the current erasing code. >> >> When I wrote the current code, I carefully checked all the corner cases. For >> example, on the first stack erasing, the STACK_END_MAGIC was not >> overwritten, but the memory next to it was erased. Same for the beginning of >> the stack: I carefully checked that no unpoisoned bytes were left on the >> thread stack. >> >>> In total, this can cause us to erase `128 + 2 * sizeof(unsigned long)` >>> bytes more than necessary, which is unfortunate. >>> >>> This patch reworks the logic to find the address immediately above the >>> poisoned region, by finding the lowest non-poisoned address. This is >>> factored into a stackleak_find_top_of_poison() helper both for clarity >>> and so that this can be shared with the LKDTM test in subsequent >>> patches. >> >> You know, I wrote stackleak_erase() in very plain C. I wanted a compiler to >> generate assembly that is very close to the original asm version. I worried >> that compilers might do weird stuff with the local variables and the stack >> pointer. >> >> So I checked stackleak for gcc versions 4.8, 5, 6, 7, 8, 9, and 10 on >> x86_64, i386 and arm64. This is my project that helped with this work: >> https://github.com/a13xp0p0v/kernel-build-containers > > I've used the kernel.org cross toolchains, as published at: > > https://mirrors.edge.kernel.org/pub/tools/crosstool/ > >> Mark, in this patch series you use many local variables and helper functions. >> Honestly, this worries me. For example, compilers can (and usually do) >> ignore the presence of the 'inline' specifier for the purpose of >> optimization. > > I've deliberately used `__always_inline` rather than regular `inline` to > prevent code being placed out-of-line. As mentioned in oether replies it has a > stronger semantic. > > Thanks, > Mark. > >> >> Thanks! >> >>> Signed-off-by: Mark Rutland >>> Cc: Alexander Popov >>> Cc: Andrew Morton >>> Cc: Andy Lutomirski >>> Cc: Kees Cook >>> --- >>> include/linux/stackleak.h | 26 ++++++++++++++++++++++++++ >>> kernel/stackleak.c | 18 ++++-------------- >>> 2 files changed, 30 insertions(+), 14 deletions(-) >>> >>> diff --git a/include/linux/stackleak.h b/include/linux/stackleak.h >>> index 467661aeb4136..c36e7a3b45e7e 100644 >>> --- a/include/linux/stackleak.h >>> +++ b/include/linux/stackleak.h >>> @@ -42,6 +42,32 @@ stackleak_task_high_bound(const struct task_struct *tsk) >>> return (unsigned long)task_pt_regs(tsk); >>> } >>> +/* >>> + * Find the address immediately above the poisoned region of the stack, where >>> + * that region falls between 'low' (inclusive) and 'high' (exclusive). >>> + */ >>> +static __always_inline unsigned long >>> +stackleak_find_top_of_poison(const unsigned long low, const unsigned long high) >>> +{ >>> + const unsigned int depth = STACKLEAK_SEARCH_DEPTH / sizeof(unsigned long); >>> + unsigned int poison_count = 0; >>> + unsigned long poison_high = high; >>> + unsigned long sp = high; >>> + >>> + while (sp > low && poison_count < depth) { >>> + sp -= sizeof(unsigned long); >>> + >>> + if (*(unsigned long *)sp == STACKLEAK_POISON) { >>> + poison_count++; >>> + } else { >>> + poison_count = 0; >>> + poison_high = sp; >>> + } >>> + } >>> + >>> + return poison_high; >>> +} >>> + >>> static inline void stackleak_task_init(struct task_struct *t) >>> { >>> t->lowest_stack = stackleak_task_low_bound(t); >>> diff --git a/kernel/stackleak.c b/kernel/stackleak.c >>> index ba346d46218f5..afd54b8e10b83 100644 >>> --- a/kernel/stackleak.c >>> +++ b/kernel/stackleak.c >>> @@ -74,20 +74,10 @@ static __always_inline void __stackleak_erase(void) >>> { >>> const unsigned long task_stack_low = stackleak_task_low_bound(current); >>> const unsigned long task_stack_high = stackleak_task_high_bound(current); >>> - 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 (erase_low > task_stack_low && poison_count <= depth) { >>> - if (*(unsigned long *)erase_low == STACKLEAK_POISON) >>> - poison_count++; >>> - else >>> - poison_count = 0; >>> - >>> - erase_low -= sizeof(unsigned long); >>> - } >>> + unsigned long erase_low, erase_high; >>> + >>> + erase_low = stackleak_find_top_of_poison(task_stack_low, >>> + current->lowest_stack); >>> #ifdef CONFIG_STACKLEAK_METRICS >>> current->prev_lowest_stack = erase_low; >> _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel