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

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

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()
> 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/CAMj1kXHiA91hH80tHFCO9QjkkfzEGZ2GJgpHnuKrusKhOULMXA@mail.gmail.com/
[1] https://lore.kernel.org/all/CANpmjNNc9vRJbD2e5DPPR8SWNSYa=MqTzniARp4UWKBUEdhh_Q@mail.gmail.com/

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.

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