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

[1] https://lore.kernel.org/all/20251117-b4-sev-gcov-ob...@google.com/

Signed-off-by: Brendan Jackman <jack...@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 <jack...@google.com>

[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 <jack...@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

[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 <jack...@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

[Marco Elver]

On Mon, 8 Dec 2025 at 02:35, Brendan Jackman <jack...@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....@google.com/
>
> [1] https://lore.kernel.org/all/20251117-b4-sev-gcov-ob...@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.

[Marco Elver]

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

[Brendan Jackman]

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?

[Marco Elver]

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

[Brendan Jackman]

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.

[Marco Elver]

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.

[Ard Biesheuvel]

(cc Kees)

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.

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

As discussed in [2], the GCOV+*SAN issue is attacked from two angles:
both adding __always_inline to the instrumentation helpers AND disabling
GCOV for noinstr.c. Only one or the other of these things is needed to
make the build error go away, but they both make sense in their own
right and both may serve to prevent other similar errors from cropping
up in future.

Signed-off-by: Brendan Jackman <jack...@google.com>
---

Changes in v2:
- Also disable GCOV for noinstr.c (i.e. squash in [0]).
- Link to v1: [2]

[0] https://lore.kernel.org/all/DERNCQGNRITE....@google.com/
[1] https://lore.kernel.org/all/20251117-b4-sev-gcov-ob...@google.com/

[2] https://lore.kernel.org/r/20251208-gcov-inline-no...@google.com

---
Brendan Jackman (3):

kasan: mark !__SANITIZE_ADDRESS__ stubs __always_inline
kcsan: mark !__SANITIZE_THREAD__ stub __always_inline

x86/sev: Disable GCOV on noinstr object

arch/x86/coco/sev/Makefile | 2 ++

include/linux/kasan-checks.h | 4 ++--
include/linux/kcsan-checks.h | 2 +-

3 files changed, 5 insertions(+), 3 deletions(-)
---
base-commit: 8f0b4cce4481fb22653697cced8d0d04027cb1e8

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

Fix this by just applying __always_inline.

Signed-off-by: Brendan Jackman <jack...@google.com>
---

[Brendan Jackman]

With Debian clang version 19.1.7 (3+build5) there are calls to
kasan_check_write() from __sev_es_nmi_complete, which violates noinstr.
Fix it by disabling GCOV for the noinstr object, as has been done for
previous such instrumentation issues.

Note that this file already disables __SANITIZE_ADDRESS__ and
__SANITIZE_THREAD__, thus calls like kasan_check_write() ought to be
nops regardless of GCOV. This has been fixed in other patches. However,
to avoid any other accidental instrumentation showing up, (and since, in
principle GCOV is instrumentation and hence should be disabled for
noinstr code anyway), disable GCOV overall as well.

Signed-off-by: Brendan Jackman <jack...@google.com>
---

arch/x86/coco/sev/Makefile | 2 ++
1 file changed, 2 insertions(+)

diff --git a/arch/x86/coco/sev/Makefile b/arch/x86/coco/sev/Makefile
index 3b8ae214a6a64de6bb208eb3b7c8bf12007ccc2c..b2e9ec2f69014fa3507d40c6c266f1b74d634fcb 100644
--- a/arch/x86/coco/sev/Makefile
+++ b/arch/x86/coco/sev/Makefile
@@ -8,3 +8,5 @@ UBSAN_SANITIZE_noinstr.o := n
# GCC may fail to respect __no_sanitize_address or __no_kcsan when inlining
KASAN_SANITIZE_noinstr.o := n
KCSAN_SANITIZE_noinstr.o := n
+
+GCOV_PROFILE_noinstr.o := n

--
2.50.1

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

Fix this by just applying __always_inline.

Signed-off-by: Brendan Jackman <jack...@google.com>
---

[Marco Elver]

It wouldn't be wrong to apply __always_inline to these 2 stub
functions as well, but I think it's fair if you just limit this to the
ones used from <linux/instrumented.h>. Either way, please
double-check.

[Marco Elver]

On Mon, 15 Dec 2025 at 11:12, Brendan Jackman <jack...@google.com> wrote:
>

Acked-by: Marco Elver <el...@google.com>

But please double check if you missed any __always_inline on stubs
(see my comment in "kcsan: mark !__SANITIZE_THREAD__ stub
__always_inline").

I don't know which tree this should go through, but since it deals
with KASAN and GCOV, perhaps -mm would be appropriate (also for the
KCSAN patch this time). There shouldn't be any conflicts with other
patches AFAIK.

Thanks,
-- Marco

[Brendan Jackman]

I was thinking here that it's a bug to call these from noinstr code,
regardless of your ambient instrumentation settings.

But yeah, on second thoughts... says who? I don't think that _has_ to be
a bug, we could totally mark them __always_inline.

[Brendan Jackman]

As discussed in [2], the GCOV+*SAN issue is attacked from two angles:
both adding __always_inline to the instrumentation helpers AND disabling
GCOV for noinstr.c. Only one or the other of these things is needed to
make the build error go away, but they both make sense in their own
right and both may serve to prevent other similar errors from cropping
up in future.

Note I have not annotated !CONFIG_* stubs, only !__SANITIZE_*__ ones.
That's because for global settings (i.e. kconfig) it remains a bug to
call these stubs from the wrong context and we'd probably like to detect
that bug even if it could be eliminated from the current build.

Concretely, the above is talking about KMSAN, i.e. stuff like
instrument_copy_from_user().

Other than that, I think everything in include/linux/instrumented.h is
covered now.

Signed-off-by: Brendan Jackman <jack...@google.com>
---

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.

Changes in v3:
- Also fix __kcsan_{dis,en}able_current()
- Link to v2: https://lore.kernel.org/r/20251215-gcov-inline-no...@google.com

Changes in v2:
- Also disable GCOV for noinstr.c (i.e. squash in [0]).
- Link to v1: [2]

[0] https://lore.kernel.org/all/DERNCQGNRITE....@google.com/
[1] https://lore.kernel.org/all/20251117-b4-sev-gcov-ob...@google.com/
[2] https://lore.kernel.org/r/20251208-gcov-inline-no...@google.com

---
Brendan Jackman (3):
kasan: mark !__SANITIZE_ADDRESS__ stubs __always_inline

kcsan: mark !__SANITIZE_THREAD__ stubs __always_inline

x86/sev: Disable GCOV on noinstr object

arch/x86/coco/sev/Makefile | 2 ++
include/linux/kasan-checks.h | 4 ++--

include/linux/kcsan-checks.h | 8 ++++----
3 files changed, 8 insertions(+), 6 deletions(-)
---
base-commit: 40fbbd64bba6c6e7a72885d2f59b6a3be9991eeb

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

Fix this by just applying __always_inline.

Signed-off-by: Brendan Jackman <jack...@google.com>
---

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

Fix this by just applying __always_inline.

Signed-off-by: Brendan Jackman <jack...@google.com>
---

include/linux/kcsan-checks.h | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/include/linux/kcsan-checks.h b/include/linux/kcsan-checks.h
index 92f3843d9ebb8177432bb4eccc151ea66d3dcbb7..c4c8e03e53459f5030ca33f9103a9bde49fd3820 100644
--- a/include/linux/kcsan-checks.h
+++ b/include/linux/kcsan-checks.h
@@ -226,10 +226,10 @@ 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,

- int type) { }
-static inline void __kcsan_enable_current(void) { }
-static inline void __kcsan_disable_current(void) { }

+static __always_inline void kcsan_check_access(const volatile void *ptr,

+ size_t size, int type) { }
+static __always_inline void __kcsan_enable_current(void) { }
+static __always_inline void __kcsan_disable_current(void) { }
#endif /* __SANITIZE_THREAD__ */

#if defined(CONFIG_KCSAN_WEAK_MEMORY) && defined(__SANITIZE_THREAD__)

--
2.50.1

[Brendan Jackman]

With Debian clang version 19.1.7 (3+build5) there are calls to
kasan_check_write() from __sev_es_nmi_complete, which violates noinstr.
Fix it by disabling GCOV for the noinstr object, as has been done for
previous such instrumentation issues.

Note that this file already disables __SANITIZE_ADDRESS__ and
__SANITIZE_THREAD__, thus calls like kasan_check_write() ought to be
nops regardless of GCOV. This has been fixed in other patches. However,
to avoid any other accidental instrumentation showing up, (and since, in
principle GCOV is instrumentation and hence should be disabled for
noinstr code anyway), disable GCOV overall as well.

Signed-off-by: Brendan Jackman <jack...@google.com>
---

[Marco Elver]

Acked-by: Marco Elver <el...@google.com>

Thanks!

[Peter Zijlstra]

On Tue, Dec 16, 2025 at 10:16:34AM +0000, Brendan Jackman wrote:
> 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.

Yeah, so don't do that? That's why we use raw_atomic_*() in things like
smp_text_poke_int3_handler().

[Brendan Jackman]

Right, this was what Ard suggested in [0]:

> For the short term, we could avoid this by using arch___set_bit()
> directly in the SEV code that triggers this issue today. But for the
> longer term, we should get write of those explicit calls to
> instrumentation intrinsics, as this is fundamentally incompatible with
> per-function overrides.

But, I think the longer term solution is actually now coming from what
Marco described in [1].

So in the meantime what's the cleanest fix? Going straight to the arch_*
calls from SEV seems pretty yucky in its own right. Adding special
un-instrumented wrappers in bitops.h seems overblown for a temporary
workaround. Meanwhile, disabling __SANITIZE_ADDRESS__ is something the
SEV code already relies on as a workaround, so if we can just make that
workaround work for this case too, it seems like a reasonable way
forward?

Anyway, I don't feel too strongly about this, I'm only pushing back
for the sake of hysteresis since I already flipflopped a couple of times
on this fix. If Ard/Marco agree with just using the arch_ functions
directly I'd be fine with that.

And in the meantime, I guess patch 3/3 is OK? As it happens, that will
already make the current error go away without needing the first 2
patches. So maybe we should just merge that and be done with it? There's
probably a good chance no other issues will show up between now and
whenever Marco's nice compiler support arrives.

[0] https://lore.kernel.org/all/CAMj1kXHiA91hH80tHFCO9Qjk...@mail.gmail.com/
[1] https://lore.kernel.org/all/CANpmjNNc9vRJbD2e5DPPR8SWNSYa=MqTzniARp4...@mail.gmail.com/

[Peter Zijlstra]

On Wed, Dec 17, 2025 at 01:53:33PM +0000, Brendan Jackman wrote:
> On Tue Dec 16, 2025 at 1:01 PM UTC, Peter Zijlstra wrote:
> > On Tue, Dec 16, 2025 at 10:16:34AM +0000, Brendan Jackman wrote:
> >> 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.
> >
> > Yeah, so don't do that? That's why we use raw_atomic_*() in things like
> > smp_text_poke_int3_handler().
>
> Right, this was what Ard suggested in [0]:
>
> > For the short term, we could avoid this by using arch___set_bit()

arch_set_bit(), right?

> > directly in the SEV code that triggers this issue today. But for the
> > longer term, we should get write of those explicit calls to
> > instrumentation intrinsics, as this is fundamentally incompatible with
> > per-function overrides.
>
> But, I think the longer term solution is actually now coming from what
> Marco described in [1].

Oh, shiny. But yeah, that is *LONG* term, as in, we can't use that until
this future CLANG (and GCC?) version becomes the minimum version we
support for sanitizer builds.

> So in the meantime what's the cleanest fix? Going straight to the arch_*
> calls from SEV seems pretty yucky in its own right.

This is what I would do (and have done in the past):

14d3b376b6c3 ("x86/entry, cpumask: Provide non-instrumented variant of cpu_is_offline()")
f5c54f77b07b ("cpumask: Add a x86-specific cpumask_clear_cpu() helper")

Now, I don't have much to say about SEV; but given Boris did that second
patch above, I'm thinking he won't be objecting too much for doing
something similar in the SEV code.

> Adding special
> un-instrumented wrappers in bitops.h seems overblown for a temporary
> workaround.

Agreed.

> Meanwhile, disabling __SANITIZE_ADDRESS__ is something the
> SEV code already relies on as a workaround, so if we can just make that
> workaround work for this case too, it seems like a reasonable way
> forward?

I'll defer to Boris on that, this is his code I think.

> Anyway, I don't feel too strongly about this, I'm only pushing back
> for the sake of hysteresis since I already flipflopped a couple of times
> on this fix. If Ard/Marco agree with just using the arch_ functions
> directly I'd be fine with that.

Yeah, fair enough :-)

> And in the meantime, I guess patch 3/3 is OK?

I'm not sure, ISTR having yelled profanities at GCOV in general for
being just straight up broken. And people seem to insist on adding more
and more *COV variants and I keep yelling at them or something.

That is GCOV, KCOV and llvm-cov are all doing basically the same damn
thing (and sure, llvm-cov gets more edges but details) and we should not
be having 3 different damn interfaces for it. Also, they all should
bloody well respect noinstr and if they don't they're just broken and
all that :-)

That is, I'd as soon just make them all "depend BROKEN" and call it a
day.

[Peter Zijlstra]

On Sat, Dec 13, 2025 at 08:59:44AM +0900, Ard Biesheuvel wrote:

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

Well, would not GCC need to grow the same thing and then we must wait
until these versions are the minimum supported versions for sanitizer
builds.

I mean, the extension is nice, but I'm afraid we can't really use it
until much later :/

[Marco Elver]

Unfortunately, yes. But let's try to get the builtin into Clang and
GCC now (for the latter, need to Cc GCC folks to help).

Then we wait for 5 years. :-)

There's a possibility to try and backport it to stable Clang and GCC
versions, but it's a long stretch (extremely unlikely).

[Segher Boessenkool]

Hi!

We (GCC) do not generally want to do backport features; even for
bugfixes the risk/reward ratio comes into the picture. It *can* be done
if some feature is important enough of course. If you have to wonder or
ask if your feature is important enough, it is not.

The reason we do not want backports of feature is it increases
maintenance cost a lot, and so, development costs as well.

I guess LLVM has a similar policy, but I of course do not speak for
them.

You might have more success getting the stuff backported to some
distro(s) you care about? Or get people to use newer compilers more
quickly of course, "five years" before people have it is pretty
ridiculous, two years is at the tail end of things already.


Segher

[Peter Zijlstra]

On Thu, Dec 18, 2025 at 05:58:44AM -0600, Segher Boessenkool wrote:

> You might have more success getting the stuff backported to some
> distro(s) you care about? Or get people to use newer compilers more
> quickly of course, "five years" before people have it is pretty
> ridiculous, two years is at the tail end of things already.

There is a difference between having and requiring it :/ Our current
minimum compiler version is gcc-8 or clang-15 (IIRC).

On the bright side, I think we can be more aggressively with compiler
versions for debug builds vs regular builds. Not being able to build a
KASAN/UBSAN/whateverSAN kernel isn't too big of a problem (IMO).

[Segher Boessenkool]

Hi!

On Thu, Dec 18, 2025 at 01:18:13PM +0100, Peter Zijlstra wrote:
> On Thu, Dec 18, 2025 at 05:58:44AM -0600, Segher Boessenkool wrote:
>
> > You might have more success getting the stuff backported to some
> > distro(s) you care about? Or get people to use newer compilers more
> > quickly of course, "five years" before people have it is pretty
> > ridiculous, two years is at the tail end of things already.
>
> There is a difference between having and requiring it :/ Our current
> minimum compiler version is gcc-8 or clang-15 (IIRC).

Very much so. If you have good reasons for requiring it, make sure you
voice that with your backport request!

Nothing we (again, GCC) do is *only* motivated by procedures. We can do
unusual things in unusual situations. But you need extraordinary
evidence for why extraordinary things would be needed, of course. Does
that apply here, you think?

> On the bright side, I think we can be more aggressively with compiler
> versions for debug builds vs regular builds. Not being able to build a
> KASAN/UBSAN/whateverSAN kernel isn't too big of a problem (IMO).

Absolutely. Just document the feature as needing a recent compiler!


Segher

[Brendan Jackman]

>>
>> So in the meantime what's the cleanest fix? Going straight to the arch_*
>> calls from SEV seems pretty yucky in its own right.
>
> This is what I would do (and have done in the past):
>
> 14d3b376b6c3 ("x86/entry, cpumask: Provide non-instrumented variant of cpu_is_offline()")
> f5c54f77b07b ("cpumask: Add a x86-specific cpumask_clear_cpu() helper")

OK, let's do it this way then.

>> > For the short term, we could avoid this by using arch___set_bit()
>
> arch_set_bit(), right?

I don't think so. Currently the GHCB accessors ar using __set_bit() i.e.
the non-atomic version. Am I missing something?

[Borislav Petkov]

On Thu, Dec 18, 2025 at 10:24:39AM +0100, Peter Zijlstra wrote:
> > And in the meantime, I guess patch 3/3 is OK?
>
> I'm not sure, ISTR having yelled profanities at GCOV in general for
> being just straight up broken. And people seem to insist on adding more
> and more *COV variants and I keep yelling at them or something.
>
> That is GCOV, KCOV and llvm-cov are all doing basically the same damn
> thing (and sure, llvm-cov gets more edges but details) and we should not
> be having 3 different damn interfaces for it. Also, they all should
> bloody well respect noinstr and if they don't they're just broken and
> all that :-)
>
> That is, I'd as soon just make them all "depend BROKEN" and call it a
> day.

Right.

And I don't think anyone has had any serious plans for COV-ing the SEV code so
lemme take 3/3.

I, like Peter, am gravitating towards a kill-that-thing-with-a-master-switch
instead of whack-a-mole-ing the kernel.

Thx.

--
Regards/Gruss,
Boris.

https://people.kernel.org/tglx/notes-about-netiquette