Thread (30 messages) 30 messages, 9 authors, 2025-08-10

Re: [PATCH v3 04/13] x86: Handle KCOV __init vs inline mismatches

From: Ard Biesheuvel <ardb@kernel.org>
Date: 2025-07-22 06:56:00
Also in: kvm, kvmarm, linux-acpi, linux-efi, linux-hardening, linux-kbuild, linux-kselftest, linux-mm, linux-riscv, linux-s390, linux-security-module, linux-trace-kernel, lkml, llvm, platform-driver-x86, sparclinux

On Tue, 22 Jul 2025 at 06:49, Kees Cook [off-list ref] wrote:
On Mon, Jul 21, 2025 at 01:14:36PM -0700, Kees Cook wrote:
quoted
On Mon, Jul 21, 2025 at 01:47:55PM +0100, Will Deacon wrote:
quoted
On Sun, Jul 20, 2025 at 04:10:01PM +1000, Ard Biesheuvel wrote:
quoted
On Sat, 19 Jul 2025 at 08:51, Kees Cook [off-list ref] wrote:
quoted
On Fri, Jul 18, 2025 at 11:36:32AM +0300, Mike Rapoport wrote:
quoted
On Thu, Jul 17, 2025 at 04:25:09PM -0700, Kees Cook wrote:
quoted
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@kernel.org>
...
quoted
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.
Got it, thanks for the explanation!
quoted
quoted
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.
Given that 'inline' is already a macro in the kernel, could we just
add __attribute__((__always_inline__)) to it when KCOV is enabled?
That sounds like a more robust approach and, by the sounds of it, we
could predicate it on GCC too. That would also provide a neat place for
a comment describing the problem.

Kees, would that work for you?
That seems like an extremely large hammer for this problem, IMO. It
feels like it could cause new strange corner cases. I'd much prefer the
small fixes I've currently got since it keeps it focused. KCOV is
already enabled for "allmodconfig", so any new instances would be found
very quickly, etc. (And GCC's fragility in this regard has already been
exposed to these cases -- it's just that I changed one of the
combinations of __init vs inline vs instrumentation.

I could give it a try, if you really prefer the big hammer approach...
I gave it a try -- it fails spectacularly. ;) Let's stick to my small
fixes instead?
Fair enough :-)
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help