All of lore.kernel.org
 help / color / mirror / Atom feed
From: Kees Cook <kees@kernel.org>
To: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Masahiro Yamada <masahiroy@kernel.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Jakub Jelinek <jakub@redhat.com>,
	Nathan Chancellor <nathan@kernel.org>,
	Nicolas Schier <nicolas@fjasle.eu>,
	Nick Desaulniers <ndesaulniers@google.com>,
	Bill Wendling <morbo@google.com>,
	Justin Stitt <justinstitt@google.com>,
	linux-kernel@vger.kernel.org, linux-kbuild@vger.kernel.org,
	llvm@lists.linux.dev, linux-hardening@vger.kernel.org
Subject: Re: [PATCH v2 2/3] stackinit: Add union initialization to selftests
Date: Tue, 4 Feb 2025 07:42:06 -0800	[thread overview]
Message-ID: <202502040737.93314491DA@keescook> (raw)
In-Reply-To: <CAMuHMdXW8VbtOAixO7w+aDOG70aZtZ50j1Ybcr8B3eYnRUcrcA@mail.gmail.com>

On Mon, Feb 03, 2025 at 03:44:38PM +0100, Geert Uytterhoeven wrote:
> Hi Kees,
> 
> On Mon, 27 Jan 2025 at 20:11, Kees Cook <kees@kernel.org> wrote:
> > The stack initialization selftests were checking scalars, strings,
> > and structs, but not unions. Add union tests (which are mostly identical
> > setup to structs). This catches the recent union initialization behavioral
> > changes seen in GCC 15. Before GCC 15, this new test passes:
> >
> >     ok 18 test_small_start_old_zero
> >
> > With GCC 15, it fails:
> >
> >     not ok 18 test_small_start_old_zero
> >
> > Specifically, a union with a larger member where a smaller member is
> > initialized with the older "= { 0 }" syntax:
> >
> > union test_small_start {
> >      char one:1;
> >      char two;
> >      short three;
> >      unsigned long four;
> >      struct big_struct {
> >              unsigned long array[8];
> >      } big;
> > };
> >
> > This is a regression in compiler behavior that Linux has depended on.
> > GCC does not seem likely to fix it, instead suggesting that affected
> > projects start using -fzero-init-padding-bits=unions:
> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=118403
> >
> > Signed-off-by: Kees Cook <kees@kernel.org>
> 
> I ran stackinit_kunit from v6.14-rc1 on m68k under ARAnyM.
> All small_start tests failed:
> 
> KTAP version 1
> 1..1
>     KTAP version 1
>     # Subtest: stackinit
>     # module: stackinit_kunit
>     1..108
>     ok 1 test_u8_zero
>     ok 2 test_u16_zero
>     ok 3 test_u32_zero
>     ok 4 test_u64_zero
>     ok 5 test_char_array_zero
>     ok 6 test_small_hole_zero
>     ok 7 test_big_hole_zero
>     ok 8 test_trailing_hole_zero
>     ok 9 test_packed_zero
>     ok 10 test_small_hole_old_zero
>     ok 11 test_big_hole_old_zero
>     ok 12 test_trailing_hole_old_zero
>     ok 13 test_packed_old_zero
>     ok 14 test_same_sizes_zero
>     # test_small_start_zero: ASSERTION FAILED at lib/stackinit_kunit.c:428
>     Expected stackinit_range_contains(fill_start, fill_size,
> target_start, target_size) to be true, but is false
> 
> stackframe was not the same between calls!? (fill 32 wide, target offset by -12)
> [...]
> m68k-linux-gnu-gcc (Ubuntu 13.3.0-6ubuntu2~24.04) 13.3.0, GNU ld (GNU Binutils for Ubuntu) 2.42

Hm, I must have some kind of misunderstanding of m68k's stack layout
(but only exposed for unions O_o). I will try to get this running in an
emulator to investigate.

-Kees

-- 
Kees Cook

  reply	other threads:[~2025-02-04 15:42 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-01-27 19:10 [PATCH v2 0/3] kbuild: Use -fzero-init-padding-bits=all Kees Cook
2025-01-27 19:10 ` [PATCH v2 1/3] stackinit: Add old-style zero-init syntax to struct tests Kees Cook
2025-01-27 19:10 ` [PATCH v2 2/3] stackinit: Add union initialization to selftests Kees Cook
2025-01-28  7:46   ` Geert Uytterhoeven
2025-02-03 14:44   ` Geert Uytterhoeven
2025-02-04 15:42     ` Kees Cook [this message]
2025-01-27 19:10 ` [PATCH v2 3/3] kbuild: Use -fzero-init-padding-bits=all Kees Cook
2025-01-27 19:54   ` Kees Cook
2025-01-30  2:35   ` Masahiro Yamada

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=202502040737.93314491DA@keescook \
    --to=kees@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=geert@linux-m68k.org \
    --cc=jakub@redhat.com \
    --cc=justinstitt@google.com \
    --cc=linux-hardening@vger.kernel.org \
    --cc=linux-kbuild@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=llvm@lists.linux.dev \
    --cc=masahiroy@kernel.org \
    --cc=morbo@google.com \
    --cc=nathan@kernel.org \
    --cc=ndesaulniers@google.com \
    --cc=nicolas@fjasle.eu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.