Received-SPF: pass (google.com: domain of linux-kernel+bounces-84124-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Date: Tue, 27 Feb 2024 14:25:33 -0800
From: Kees Cook <keescook@chromium.org>
To: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Guenter Roeck <linux@roeck-us.net>, linux-m68k@lists.linux-m68k.org,
	linux-kernel@vger.kernel.org
Subject: Re: stackinit unit test failures on m68k
Message-ID: <202402271423.270DBF1@keescook>
References: <a0d10d50-2720-4ecd-a2c6-c2c5e5aeee65@roeck-us.net>
 <CAMuHMdXMsxRRMV8g6+9vTy_4o8HF49SUh2deNdFjgKwDLEWrxQ@mail.gmail.com>
 <202402271401.CB43AB2E8@keescook>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <202402271401.CB43AB2E8@keescook>

On Tue, Feb 27, 2024 at 02:19:07PM -0800, Kees Cook wrote:
> It is basically doing this:
> 
> static void *fill_start, *target_start;
> static size_t fill_size, target_size;
> 
> static noinline int leaf_char_array_none(unsigned long sp, bool fill,
>                                   unsigned char *arg)
> {
>         char buf[32];
>         unsigned char var[16];
> 
>         target_start = &var;
>         target_size = sizeof(var);
>         /*
>          * Keep this buffer around to make sure we've got a
>          * stack frame of SOME kind...
>          */
>         memset(buf, (char)(sp & 0xff), sizeof(buf));
>         /* Fill variable with 0xFF. */
>         if (fill) {
>                 fill_start = &var;
>                 fill_size = sizeof(var);
>                 memset(fill_start,
>                        (char)((sp & 0xff) | forced_mask),
>                        fill_size);
>         }
> 
>         /* Silence "never initialized" warnings. */
> 	do_nothing_char_array(var);
> 
>         /* Exfiltrate "var". */
>         memcpy(check_buf, target_start, target_size);
> 
>         return (int)buf[0] | (int)buf[sizeof(buf) - 1];
> }
> 
> and it's called as:
> 
> 
>         ignored = leaf_char_array_none((unsigned long)&ignored, 1, zero);
> 	...
>         ignored = leaf_char_array_none((unsigned long)&ignored, 0, zero);
> 
> The first call remembers where "var" is in the stack frame via the
> fill_start assignment, and the second call records where "var" is via
> the target_start assignment.
> 
> The complaint is that it _changes_ between the two calls. ... Oh, I
> think I see what's happened. Between the two calls, the stack grows (and
> is for some reason not reclaimed) due to the KUNIT checks between the two
> leaf calls. Yes, moving that fixes it.
> 
> I'll send a patch!

Oh, no, that wasn't it. Something else is happening. The stack pointer
isn't moving between them. Is there anything special about character
arrays on m68k?

-- 
Kees Cook