Re: [PATCH 0/2] Noinstr fixes for K[CA]SAN with GCOV

[Marco Elver]

On Tue, 9 Dec 2025 at 03:25, Brendan Jackman <jackmanb@xxxxxxxxxx> wrote:
> On Tue Dec 9, 2025 at 12:52 AM UTC, Marco Elver wrote:
> > On Tue, 9 Dec 2025 at 01:05, Brendan Jackman <jackmanb@xxxxxxxxxx> wrote:
> >> On Mon Dec 8, 2025 at 11:12 AM UTC, Marco Elver wrote:
> >> > On Mon, 8 Dec 2025 at 10:37, Marco Elver <elver@xxxxxxxxxx> wrote:
> >> >>
> >> >> On Mon, 8 Dec 2025 at 02:35, Brendan Jackman <jackmanb@xxxxxxxxxx> wrote:
> >> >> >
> >> >> > Details:
> >> >> >
> >> >> >  - ❯❯  clang --version
> >> >> >    Debian clang version 19.1.7 (3+build5)
> >> >> >    Target: x86_64-pc-linux-gnu
> >> >> >    Thread model: posix
> >> >> >    InstalledDir: /usr/lib/llvm-19/bin
> >> >> >
> >> >> >  - Kernel config:
> >> >> >
> >> >> >    https://gist.githubusercontent.com/bjackman/bbfdf4ec2e1dfd0e18657174f0537e2c/raw/a88dcc6567d14c69445e7928a7d5dfc23ca9f619/gistfile0.txt
> >> >> >
> >> >> > Note I also get this error:
> >> >> >
> >> >> > vmlinux.o: warning: objtool: set_ftrace_ops_ro+0x3b: relocation to !ENDBR: machine_kexec_prepare+0x810
> >> >> >
> >> >> > That one's a total mystery to me. I guess it's better to "fix" the SEV
> >> >> > one independently rather than waiting until I know how to fix them both.
> >> >> >
> >> >> > Note I also mentioned other similar errors in [0]. Those errors don't
> >> >> > exist in Linus' master and I didn't note down where I saw them. Either
> >> >> > they have since been fixed, or I observed them in Google's internal
> >> >> > codebase where they were instroduced downstream.
> >> >> >
> >> >> > This is a successor to [1] but I haven't called it a v2 because it's a
> >> >> > totally different solution. Thanks to Ard for the guidance and
> >> >> > corrections.
> >> >> >
> >> >> > [0] https://lore.kernel.org/all/DERNCQGNRITE.139O331ACPKZ9@xxxxxxxxxx/
> >> >> >
> >> >> > [1] https://lore.kernel.org/all/20251117-b4-sev-gcov-objtool-v1-1-54f7790d54df@xxxxxxxxxx/
> >> >>
> >> >> Why is [1] not the right solution?
> >> >> The problem is we have lots of "inline" functions, and any one of them
> >> >> could cause problems in future.
> >> >
> >> > Perhaps I should qualify: lots of *small* inline functions, including
> >> > those stubs.
> >> >
> >> >> I don't mind turning "inline" into "__always_inline", but it seems
> >> >> we're playing whack-a-mole here, and just disabling GCOV entirely
> >> >> would make this noinstr.c file more robust.
> >> >
> >> > To elaborate: `UBSAN_SANITIZE_noinstr.o := n` and
> >> > `K{A,C}SAN_SANITIZE_noinstr.o := n` is already set on this file.
> >> > Perhaps adding __always_inline to the stub functions here will be
> >> > enough today, but might no longer be in future.
> >>
> >> Well you can also see it the other way around: disabling GCOV_PROFILE
> >> might be enough today, but as soon as some other noinstr disables
> >> __SANITIZE_ADDRESS__ and expects to be able to call instrumented
> >> helpers, that code will be broken too.
> >
> > This itself is a contradiction: a `noinstr` function should not call
> > instrumented helpers. Normally this all works due to the compiler's
> > function attributes working as intended for the compiler-inserted
> > instrumentation, but for explicitly inserted instrumentation it's
> > obviously not. In otherwise instrumented files with few (not all)
> > `noinstr` functions, making the stub functions `__always_inline` will
> > not work, because the preprocessor is applied globally not per
> > function. In the past, I recall the underlying implementation being
> > used of e.g. the bitops (arch_foo... or __foo) in `noinstr` functions
> > to solve that.
>
> Sorry I dropped an important word here, I meant to say other noinstr
> _files_. I.e. anything else similar to SEV's noinstr.c that is doing
> noinstr at the file level.

Someone at LPC (I couldn't make out who due to technical difficulties)
mentioned that calling explicitly instrumented helpers from noinstr
functions is a general problem.

After that I sat down and finally got around to implement the builtin
that should solve this once and for all, regardless of where it's
called: https://github.com/llvm/llvm-project/pull/172030
What this will allow us to do is to remove the
"K[AC]SAN_SANITIZE_noinstr.o := n" lines from the Makefile, and purely
rely on the noinstr attribute, even in the presence of explicit
instrumentation calls.

It will be a while until it's available and the kernel needs patches
to use it, too, but that should be easy enough. Once it lands in
Clang, it'd be nice if GCC could provide the same.

So for the time being, we still need your patches here to work around
the problem.