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

[Brendan Jackman <jackmanb@google.com>]

On Mon Dec 8, 2025 at 11:12 AM UTC, Marco Elver wrote:
> On Mon, 8 Dec 2025 at 10:37, Marco Elver <elver@google.com> wrote:
>>
>> On Mon, 8 Dec 2025 at 02:35, Brendan Jackman <jackmanb@google.com> 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@google.com/
>> >
>> > [1] https://lore.kernel.org/all/20251117-b4-sev-gcov-objtool-v1-1-54f7790d54df@google.com/
>>
>> 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. 

I don't think we can avoid whack-a-mole here. In fact I think the whole
noinstr thing is an inevitable game of whack-a-mole unless we can get a
static anlyzer to find violations at the source level. I suspect there
are loads of violations in the tree that only show up in objtool if you
build in weird configs on a full moon.

One argument in favour of `GCOV_PROFILE_noinstr.o := n` would be: "this
is non-instrumentable code, the issue here is that it is getting
instrumented, so the fix is surely to stop instrumenting it". But, I
don't think that's really true, the issue is not with the
instrumentation but with the out-of-lining. Which highlights another
point: a sufficiently annoying compiler could out-of-line these
stub functions even without GCOV, right?

Still, despite my long-winded arguments I'm not gonna die on this hill,
I would be OK with both ways.

> If you look at
> <linux/instrumented.h>, we also have KMSAN. The KMSAN explicit
> instrumentation doesn't appear to be invoked on that file today, but
> given it shouldn't, we might consider:
>
> KMSAN_SANITIZE_noinstr.o := n
> GCOV_PROFILE_noinstr.o := n

This would make sense to me, although as I hinted above I think it's
sorta orthogonal and we should __always_inline the k[ca]san stubs
regardless.

> The alternative is to audit the various sanitizer stub functions, and
> mark all these "inline" stub functions as "__always_inline". The
> changes made in this series are sufficient for the noinstr.c case, but
> not complete.

Oh, yeah I should have  done __kcsan_{en,di}able_current() too I think.

Are there other stubs you are thinking of? I think we only care about the
!__SANITIZE_*__ stubs - we don't need this for !CONFIG_* stubs, right?
Anything else I'm forgetting?