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

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

[Brendan Jackman]

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/

Signed-off-by: Brendan Jackman <jackmanb@google.com>
---
Brendan Jackman (2):
      kasan: mark !__SANITIZE_ADDRESS__ stubs __always_inline
      kcsan: mark !__SANITIZE_THREAD__ stub __always_inline

 include/linux/kasan-checks.h | 4 ++--
 include/linux/kcsan-checks.h | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)
---
base-commit: 67a454e6b1c604555c04501c77b7fedc5d98a779
change-id: 20251208-gcov-inline-noinstr-1550cfee445c

Best regards,
-- 
Brendan Jackman <jackmanb@google.com>

[PATCH 1/2] kasan: mark !__SANITIZE_ADDRESS__ stubs __always_inline

[Brendan Jackman]

The x86 instrumented bitops in
include/asm-generic/bitops/instrumented-non-atomic.h are
KASAN-instrumented via explicit calls to instrument_* functions from
include/linux/instrumented.h.

This bitops are used from noinstr code in __sev_es_nmi_complete(). This
code avoids noinstr violations by disabling __SANITIZE_ADDRESS__ etc for
the compilation unit.

However, when GCOV is enabled, there can still be violations caused by
the stub versions of these functions, since coverage instrumentation is
injected that causes them to be out-of-lined.

(Note: the GCOV isntrumentation itself also appears to violate noinstr
in principle, but it appears to be harmless - basically just an inc
instruction).

Fix this by just applying __always_inline.

Signed-off-by: Brendan Jackman <jackmanb@google.com>
---
 include/linux/kasan-checks.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/linux/kasan-checks.h b/include/linux/kasan-checks.h
index 3d6d22a25bdc391c0015a6daf2249d6bea752dcb..9aa0f1cc90133ca334afa478b5f762aef9e5d79c 100644
--- a/include/linux/kasan-checks.h
+++ b/include/linux/kasan-checks.h
@@ -37,11 +37,11 @@ static inline bool __kasan_check_write(const volatile void *p, unsigned int size
 #define kasan_check_read __kasan_check_read
 #define kasan_check_write __kasan_check_write
 #else
-static inline bool kasan_check_read(const volatile void *p, unsigned int size)
+static __always_inline bool kasan_check_read(const volatile void *p, unsigned int size)
 {
 	return true;
 }
-static inline bool kasan_check_write(const volatile void *p, unsigned int size)
+static __always_inline bool kasan_check_write(const volatile void *p, unsigned int size)
 {
 	return true;
 }

-- 
2.50.1

[PATCH 2/2] kcsan: mark !__SANITIZE_THREAD__ stub __always_inline

[Brendan Jackman]

The x86 instrumented bitops in
include/asm-generic/bitops/instrumented-non-atomic.h are
KCSAN-instrumented via explicit calls to instrument_* functions from
include/linux/instrumented.h.

This bitops are used from noinstr code in __sev_es_nmi_complete(). This
code avoids noinstr violations by disabling __SANITIZE_THREAD__ etc for
the compilation unit.

However, when GCOV is enabled, there can still be violations caused by
the stub versions of these functions, since coverage instrumentation is
injected that causes them to be out-of-lined.

(Note: the GCOV isntrumentation itself also appears to violate noinstr
in principle, but it appears to be harmless - basically just an inc
instruction).

Fix this by just applying __always_inline.

Signed-off-by: Brendan Jackman <jackmanb@google.com>
---
 include/linux/kcsan-checks.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/kcsan-checks.h b/include/linux/kcsan-checks.h
index 92f3843d9ebb8177432bb4eccc151ea66d3dcbb7..cabb2ae46bdc0963bd89533777cab586ab4d5a1b 100644
--- a/include/linux/kcsan-checks.h
+++ b/include/linux/kcsan-checks.h
@@ -226,7 +226,7 @@ static inline void kcsan_end_scoped_access(struct kcsan_scoped_access *sa) { }
 #define __kcsan_disable_current kcsan_disable_current
 #define __kcsan_enable_current kcsan_enable_current_nowarn
 #else /* __SANITIZE_THREAD__ */
-static inline void kcsan_check_access(const volatile void *ptr, size_t size,
+static __always_inline void kcsan_check_access(const volatile void *ptr, size_t size,
 				      int type) { }
 static inline void __kcsan_enable_current(void)  { }
 static inline void __kcsan_disable_current(void) { }

-- 
2.50.1

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

[Marco Elver]

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.

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.

> Signed-off-by: Brendan Jackman <jackmanb@google.com>
> ---
> Brendan Jackman (2):
>       kasan: mark !__SANITIZE_ADDRESS__ stubs __always_inline
>       kcsan: mark !__SANITIZE_THREAD__ stub __always_inline
>
>  include/linux/kasan-checks.h | 4 ++--
>  include/linux/kcsan-checks.h | 2 +-
>  2 files changed, 3 insertions(+), 3 deletions(-)
> ---
> base-commit: 67a454e6b1c604555c04501c77b7fedc5d98a779
> change-id: 20251208-gcov-inline-noinstr-1550cfee445c
>
> Best regards,
> --
> Brendan Jackman <jackmanb@google.com>
>

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

[Marco Elver]

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. 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

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.

Thanks,
-- Marco

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

[Brendan Jackman]

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?

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

[Marco Elver]

On Tue, 9 Dec 2025 at 01:05, Brendan Jackman <jackmanb@google.com> 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@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.

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.

The other hammer we have is K[ACM]SAN_SANITIZE_foo.o := n,
GCOV_PROFILE_foo.o := n, and KCOV_INSTRUMENT_foo.o := n.

> 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?

This would be a compiler bug in my book. Without instrumentation a
"static inline" function with nothing in it not being inlined is an
optimization bug. But those things get caught because it'd have made
someone's system slow.

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

To some extent I think doing both to reduce the chance of issues in
future might be what you want. On the other hand, avoiding the
Makefile-level opt-out will help catch more corner cases in future,
which may or may not be helpful outside this noinstr.c file.

> > 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?

Initially, I think !__SANITIZE_* stubs are enough. Well, basically
anything that appears in <linux/instrumented.h>, because all those are
__always_inline, we should make the called functions also
__always_inline.

If you think that'll be enough, and the Makefile-level opt-out is
redundant as a result, let's just do that.

Thanks,
-- Marco

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

[Brendan Jackman]

On Tue Dec 9, 2025 at 12:52 AM UTC, Marco Elver wrote:
> On Tue, 9 Dec 2025 at 01:05, Brendan Jackman <jackmanb@google.com> 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@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.
>
> 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.

>> Still, despite my long-winded arguments I'm not gonna die on this hill,
>> I would be OK with both ways.
>
> To some extent I think doing both to reduce the chance of issues in
> future might be what you want. On the other hand, avoiding the
> Makefile-level opt-out will help catch more corner cases in future,
> which may or may not be helpful outside this noinstr.c file.

Cool, then yeah I think I will do both unless anyone shows up to object
to that. Both things ultimately make sense on their own merit and even
if you only need one or the other to make the error go away, I don't
think that actually makes them "redundant".

>> > 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?
>
> Initially, I think !__SANITIZE_* stubs are enough. Well, basically
> anything that appears in <linux/instrumented.h>, because all those are
> __always_inline, we should make the called functions also
> __always_inline.

Ack, thanks for all the input here! I'll probably wait until after LPC
to do a v2.

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@google.com> 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@google.com> 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@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.
> >
> > 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.

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

[Ard Biesheuvel]

(cc Kees)

On Sat, 13 Dec 2025 at 01:11, Marco Elver <elver@google.com> wrote:
>
> On Tue, 9 Dec 2025 at 03:25, Brendan Jackman <jackmanb@google.com> 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@google.com> 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@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.
> > >
> > > 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.
>

That was me.

> 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.
>

Excellent! Thanks for the quick fix. Happy to test and/or look into
the kernel side of this once this lands.