linux-kernel - Re: [PATCH v2 05/17] vdso: Avoid call to memset() by getrandom

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAMj1kXGmDmxy75eP=rf_fzKmg0g_FeKV43jk2G_gibnKZBtVww@mail.gmail.com>
Date: Wed, 28 Aug 2024 17:40:23 +0200
From: Ard Biesheuvel <ardb@...nel.org>
To: Segher Boessenkool <segher@...nel.crashing.org>
Cc: Arnd Bergmann <arnd@...db.de>, "Jason A . Donenfeld" <Jason@...c4.com>, Eric Biggers <ebiggers@...nel.org>, 
	Christophe Leroy <christophe.leroy@...roup.eu>, Michael Ellerman <mpe@...erman.id.au>, 
	Nicholas Piggin <npiggin@...il.com>, Naveen N Rao <naveen@...nel.org>, Andy Lutomirski <luto@...nel.org>, 
	Thomas Gleixner <tglx@...utronix.de>, Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>, 
	Dave Hansen <dave.hansen@...ux.intel.com>, x86@...nel.org, 
	"H. Peter Anvin" <hpa@...or.com>, "Theodore Ts'o" <tytso@....edu>, Andrew Morton <akpm@...ux-foundation.org>, 
	Steven Rostedt <rostedt@...dmis.org>, Masami Hiramatsu <mhiramat@...nel.org>, 
	Mathieu Desnoyers <mathieu.desnoyers@...icios.com>, 
	Vincenzo Frascino <vincenzo.frascino@....com>, shuah <shuah@...nel.org>, 
	linuxppc-dev@...ts.ozlabs.org, linux-kernel@...r.kernel.org, 
	linux-fsdevel@...r.kernel.org, Linux-Arch <linux-arch@...r.kernel.org>, 
	linux-mm@...ck.org, linux-trace-kernel@...r.kernel.org, 
	linux-kselftest@...r.kernel.org
Subject: Re: [PATCH v2 05/17] vdso: Avoid call to memset() by getrandom

On Wed, 28 Aug 2024 at 14:57, Segher Boessenkool
<segher@...nel.crashing.org> wrote:
>
> On Wed, Aug 28, 2024 at 12:24:12PM +0000, Arnd Bergmann wrote:
> > On Wed, Aug 28, 2024, at 11:18, Jason A. Donenfeld wrote:
> > > On Tue, Aug 27, 2024 at 05:53:30PM -0500, Segher Boessenkool wrote:
> > >> On Tue, Aug 27, 2024 at 11:08:19AM -0700, Eric Biggers wrote:
> > >> >
> > >> > Is there a compiler flag that could be used to disable the generation of calls
> > >> > to memset?
> > >>
> > >> -fno-tree-loop-distribute-patterns .  But, as always, read up on it, see
> > >> what it actually does (and how it avoids your problem, and mostly: learn
> > >> what the actual problem *was*!)
> > >
> > > This might help with various loops, but it doesn't help with the matter
> > > that this patch fixes, which is struct initialization. I just tried it
> > > with the arm64 patch to no avail.
> >
> > Maybe -ffreestanding can help here? That should cause the vdso to be built
> > with the assumption that there is no libc, so it would neither add nor
> > remove standard library calls. Not sure if that causes other problems,
> > e.g. if the calling conventions are different.
>
> "GCC requires the freestanding
> environment provide 'memcpy', 'memmove', 'memset' and 'memcmp'."
>
> This is precisely to implement things like struct initialisation.  Maybe
> we should have a "-ffreeerstanding" or "-ffreefloating" or think of
> something funnier still environment as well, this problem has been there
> since the -ffreestanding flag has existed, but the problem is as old as
> the night.
>
> -fno-builtin might help a bit more, but just attack the problem at
> its root, like I suggested?
>

In my experience, this is likely to do the opposite: it causes the
compiler to 'forget' the semantics of memcpy() and memset(), so that
explicit trivial calls will no longer be elided and replaced with
plain loads and stores (as it can no longer guarantee the equivalence)

> (This isn't a new problem, originally it showed up as "GCC replaces
> (part of) my memcpy() implementation by a (recursive) call to memcpy()"
> and, well, that doesn't quite work!)
>

This needs to be fixed for Clang as well, so throwing GCC specific
flags at it will at best be a partial solution.

Omitting the struct assignment is a reasonable way to reduce the
likelihood that a memset() will be emitted, so for this patch

Acked-by: Ard Biesheuvel <ardb@...nel.org>

It is not a complete solution, unfortunately, and I guess there may be
other situations (compiler/arch combinations) where this might pop up
again.