Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Wed, 4 May 2022 12:07:57 -0700
From:   Kees Cook <keescook@chromium.org>
To:     Mark Rutland <mark.rutland@arm.com>
Cc:     linux-arm-kernel@lists.infradead.org, akpm@linux-foundation.org,
        alex.popov@linux.com, catalin.marinas@arm.com,
        linux-kernel@vger.kernel.org, luto@kernel.org, will@kernel.org
Subject: Re: [PATCH v2 09/13] lkdtm/stackleak: rework boundary management
Message-ID: <202205041205.803FD491@keescook>
References: <20220427173128.2603085-1-mark.rutland@arm.com>
 <20220427173128.2603085-10-mark.rutland@arm.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20220427173128.2603085-10-mark.rutland@arm.com>
Precedence: bulk

On Wed, Apr 27, 2022 at 06:31:24PM +0100, Mark Rutland wrote:
> There are a few problems with the way the LKDTM STACKLEAK_ERASING test
> manipulates the stack pointer and boundary values:
> 
> * It uses the address of a local variable to determine the current stack
>   pointer, rather than using current_stack_pointer directly. As the
>   local variable could be placed anywhere within the stack frame, this
>   can be an over-estimate of the true stack pointer value.
> 
> * Is uses an estiamte of the current stack pointer as the upper boundary
>   when scanning for poison, even though prior functions could have used
>   more stack (and may have updated current->lowest stack accordingly).
> 
> * A pr_info() call is made in the middle of the test. As the printk()
>   code is out-of-line and will make use of the stack, this could clobber
>   poison and/or adjust current->lowest_stack. It would be better to log
>   the metadata after the body of the test to avoid such problems.

Yeah, I noticed this too when I was testing the v1 series. I started
cleaning it up, but your version is much better. :)

-- 
Kees Cook