2024-02-11 23:06:58

by Guenter Roeck

[permalink] [raw]
Subject: stackinit unit test failures on m68k

Hi Geert,

I see the following stackinit unit test failures on m68k when running
the q800 emulation.

# test_char_array_zero: ASSERTION FAILED at lib/stackinit_kunit.c:333
Expected stackinit_range_contains(fill_start, fill_size, target_start, target_size) to be true, but is false
stack fill missed target!? (fill 16 wide, target offset by -12)

# test_char_array_none: ASSERTION FAILED at lib/stackinit_kunit.c:343
Expected stackinit_range_contains(fill_start, fill_size, target_start, target_size) to be true, but is false
stack fill missed target!? (fill 16 wide, target offset by -12)

Do you happen to know if this a problem with the test, with m68k, or maybe
with the configuration ? My configuration is based on mac_defconfig with
various test options enabled. I use gcc 11.4 to build the image. I tried
with qemu v8.1 and v8.2.

Thanks,
Guenter


2024-02-12 08:34:32

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: stackinit unit test failures on m68k

Hi Günter,

On Mon, Feb 12, 2024 at 12:06 AM Guenter Roeck <[email protected]> wrote:
> I see the following stackinit unit test failures on m68k when running
> the q800 emulation.
>
> # test_char_array_zero: ASSERTION FAILED at lib/stackinit_kunit.c:333
> Expected stackinit_range_contains(fill_start, fill_size, target_start, target_size) to be true, but is false
> stack fill missed target!? (fill 16 wide, target offset by -12)
>
> # test_char_array_none: ASSERTION FAILED at lib/stackinit_kunit.c:343
> Expected stackinit_range_contains(fill_start, fill_size, target_start, target_size) to be true, but is false
> stack fill missed target!? (fill 16 wide, target offset by -12)
>
> Do you happen to know if this a problem with the test, with m68k, or maybe
> with the configuration ? My configuration is based on mac_defconfig with
> various test options enabled. I use gcc 11.4 to build the image. I tried
> with qemu v8.1 and v8.2.

Thanks, I see the same failures in the logs of my last testrun on ARAnyM, too.
I haven't looked into the details yet.

Only two failures does look like a nice improvement, compared to the
previous time I ran that test ;-)

https://lore.kernel.org/all/CAMuHMdX_g1tbiUL9PUQdqaegrEzCNN3GtbSvSBFYAL4TzvstFg@mail.gmail.com

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68korg

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

2024-02-27 22:19:18

by Kees Cook

[permalink] [raw]
Subject: Re: stackinit unit test failures on m68k

On Mon, Feb 12, 2024 at 09:34:02AM +0100, Geert Uytterhoeven wrote:
> Hi Günter,
>
> On Mon, Feb 12, 2024 at 12:06 AM Guenter Roeck <[email protected]> wrote:
> > I see the following stackinit unit test failures on m68k when running
> > the q800 emulation.
> >
> > # test_char_array_zero: ASSERTION FAILED at lib/stackinit_kunit.c:333
> > Expected stackinit_range_contains(fill_start, fill_size, target_start, target_size) to be true, but is false
> > stack fill missed target!? (fill 16 wide, target offset by -12)
> >
> > # test_char_array_none: ASSERTION FAILED at lib/stackinit_kunit.c:343
> > Expected stackinit_range_contains(fill_start, fill_size, target_start, target_size) to be true, but is false
> > stack fill missed target!? (fill 16 wide, target offset by -12)
> >
> > Do you happen to know if this a problem with the test, with m68k, or maybe
> > with the configuration ? My configuration is based on mac_defconfig with
> > various test options enabled. I use gcc 11.4 to build the image. I tried
> > with qemu v8.1 and v8.2.
>
> Thanks, I see the same failures in the logs of my last testrun on ARAnyM, too.
> I haven't looked into the details yet.
>
> Only two failures does look like a nice improvement, compared to the
> previous time I ran that test ;-)
>
> https://lore.kernel.org/all/CAMuHMdX_g1tbiUL9PUQdqaegrEzCNN3GtbSvSBFYAL4TzvstFg@mail.gmail.com

This is complaining that the stack frames across subsequent calls to the
same leaf function don't end up putting the same variable in the same
place.

It's a rather difficult set of macros used try many different
combinations, but it's specifically talking about the "leaf_..."
function at line 208 of lib/stackinit_kunit.c. This test passes for all
the integral types, but seems to fail for a character array.

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!

-Kees

--
Kees Cook

2024-02-27 22:25:43

by Kees Cook

[permalink] [raw]
Subject: Re: stackinit unit test failures on m68k

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

2024-02-27 22:54:30

by Guenter Roeck

[permalink] [raw]
Subject: Re: stackinit unit test failures on m68k

On 2/27/24 14:33, Finn Thain wrote:
>
> On Tue, 27 Feb 2024, Kees Cook wrote:
>
>>
>> I'll send a patch!
>>
>
> Or you could just not run the test on m68k. It's said that, "What the eye
> does not see, the heart does not grieve over." Is that not true for bugs?

Yes, that is exactly what I am doing now. Problem "solved".

Guenter


2024-02-27 22:58:25

by Andreas Schwab

[permalink] [raw]
Subject: Re: stackinit unit test failures on m68k

On Feb 27 2024, Kees Cook wrote:

> 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?

You mean, beyond undefined behaviour?

--
Andreas Schwab, [email protected]
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1
"And now for something completely different."

2024-02-28 00:55:12

by Finn Thain

[permalink] [raw]
Subject: Re: stackinit unit test failures on m68k


On Tue, 27 Feb 2024, Kees Cook wrote:

>
> I'll send a patch!
>

Or you could just not run the test on m68k. It's said that, "What the eye
does not see, the heart does not grieve over." Is that not true for bugs?

2024-02-29 22:42:04

by David Laight

[permalink] [raw]
Subject: RE: stackinit unit test failures on m68k

...
> 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.

Is the noinline enough to stop gcc generating two copies of the
function for the different values of 'fill'?

You might need to call through a volatile global function pointer
variable?

David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)