linux-kernel - Re: [PATCH v3 04/13] x86: Handle KCOV _

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: <202507181541.B8CFAC7E@keescook>
Date: Fri, 18 Jul 2025 15:51:28 -0700
From: Kees Cook <kees@...nel.org>
To: Mike Rapoport <rppt@...nel.org>, Will Deacon <will@...nel.org>
Cc: Arnd Bergmann <arnd@...db.de>, 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>,
	Paolo Bonzini <pbonzini@...hat.com>,
	Vitaly Kuznetsov <vkuznets@...hat.com>,
	Henrique de Moraes Holschuh <hmh@....eng.br>,
	Hans de Goede <hdegoede@...hat.com>,
	Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>,
	"Rafael J. Wysocki" <rafael@...nel.org>,
	Len Brown <lenb@...nel.org>, Masami Hiramatsu <mhiramat@...nel.org>,
	Ard Biesheuvel <ardb@...nel.org>,
	Michal Wilczynski <michal.wilczynski@...el.com>,
	Juergen Gross <jgross@...e.com>,
	Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
	"Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>,
	Roger Pau Monne <roger.pau@...rix.com>,
	David Woodhouse <dwmw@...zon.co.uk>,
	Usama Arif <usama.arif@...edance.com>,
	"Guilherme G. Piccoli" <gpiccoli@...lia.com>,
	Thomas Huth <thuth@...hat.com>, Brian Gerst <brgerst@...il.com>,
	kvm@...r.kernel.org, ibm-acpi-devel@...ts.sourceforge.net,
	platform-driver-x86@...r.kernel.org, linux-acpi@...r.kernel.org,
	linux-trace-kernel@...r.kernel.org, linux-efi@...r.kernel.org,
	linux-mm@...ck.org, Ingo Molnar <mingo@...nel.org>,
	"Gustavo A. R. Silva" <gustavoars@...nel.org>,
	Christoph Hellwig <hch@....de>,
	Andrey Konovalov <andreyknvl@...il.com>,
	Andrey Ryabinin <ryabinin.a.a@...il.com>,
	Masahiro Yamada <masahiroy@...nel.org>,
	Nathan Chancellor <nathan@...nel.org>,
	Nicolas Schier <nicolas.schier@...ux.dev>,
	Nick Desaulniers <nick.desaulniers+lkml@...il.com>,
	Bill Wendling <morbo@...gle.com>,
	Justin Stitt <justinstitt@...gle.com>, linux-kernel@...r.kernel.org,
	kasan-dev@...glegroups.com, linux-doc@...r.kernel.org,
	linux-arm-kernel@...ts.infradead.org, kvmarm@...ts.linux.dev,
	linux-riscv@...ts.infradead.org, linux-s390@...r.kernel.org,
	linux-hardening@...r.kernel.org, linux-kbuild@...r.kernel.org,
	linux-security-module@...r.kernel.org,
	linux-kselftest@...r.kernel.org, sparclinux@...r.kernel.org,
	llvm@...ts.linux.dev
Subject: Re: [PATCH v3 04/13] x86: Handle KCOV __init vs inline mismatches

On Fri, Jul 18, 2025 at 11:36:32AM +0300, Mike Rapoport wrote:
> Hi Kees,
> 
> On Thu, Jul 17, 2025 at 04:25:09PM -0700, Kees Cook wrote:
> > When KCOV is enabled all functions get instrumented, unless the
> > __no_sanitize_coverage attribute is used. To prepare for
> > __no_sanitize_coverage being applied to __init functions, we have to
> > handle differences in how GCC's inline optimizations get resolved. For
> > x86 this means forcing several functions to be inline with
> > __always_inline.
> > 
> > Signed-off-by: Kees Cook <kees@...nel.org>
> 
> ...
> 
> > diff --git a/include/linux/memblock.h b/include/linux/memblock.h
> > index bb19a2534224..b96746376e17 100644
> > --- a/include/linux/memblock.h
> > +++ b/include/linux/memblock.h
> > @@ -463,7 +463,7 @@ static inline void *memblock_alloc_raw(phys_addr_t size,
> >  					  NUMA_NO_NODE);
> >  }
> >  
> > -static inline void *memblock_alloc_from(phys_addr_t size,
> > +static __always_inline void *memblock_alloc_from(phys_addr_t size,
> >  						phys_addr_t align,
> >  						phys_addr_t min_addr)
> 
> I'm curious why from all memblock_alloc* wrappers this is the only one that
> needs to be __always_inline?

Thread-merge[1], adding Will Deacon, who was kind of asking the same
question.

Based on what I can tell, GCC has kind of fragile inlining logic, in the
sense that it can change whether or not it inlines something based on
optimizations. It looks like the kcov instrumentation being added (or in
this case, removed) from a function changes the optimization results,
and some functions marked "inline" are _not_ inlined. In that case, we end up
with __init code calling a function not marked __init, and we get the
build warnings I'm trying to eliminate.

So, to Will's comment, yes, the problem is somewhat fragile (though
using either __always_inline or __init will deterministically solve it).
We've tripped over this before with GCC and the solution has usually
been to just use __always_inline and move on.

For memblock_alloc*, it appears to be that the heuristic GCC uses
resulted in only memblock_alloc_from() being a problem in this case. I
can certainly mark them all as __always_inline if that is preferred.

Some maintainers have wanted things marked __init, some have wanted
__always_inline. I opted for __always_inline since that was basically
the intent of marking a function "inline" in the first place. I am happy
to do whatever. :)

-Kees

[1] https://lore.kernel.org/lkml/aHouXI5-tyQw78Ht@willie-the-truck/

-- 
Kees Cook