Re: [PATCH 4/4] x86/percpu: Use C for percpu read/write accessors

[Uros Bizjak <ubizjak@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 2559 bytes --]

On Sun, Oct 8, 2023 at 10:48 PM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> On Sun, 8 Oct 2023 at 13:13, Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
> >
> > Your dump does end up being close to a %gs access:
>
> Bah. I should have looked closer at the instructions before the oops.
>
> Because I think that's exactly the problem here. That's the KASAN
> checks that have been added, and we have this insane code:
>
> >   10: 48 c7 c0 10 73 02 00 mov    $0x27310,%rax
> >   17: 48 ba 00 00 00 00 00 movabs $0xdffffc0000000000,%rdx
> >   1e: fc ff df
> >   21: 48 c1 e8 03          shr    $0x3,%rax
> >   25:* 80 3c 10 00          cmpb   $0x0,(%rax,%rdx,1) <-- trapping instruction
>
> Look how both %rax and %rdx are constants, yet then gcc has generated
> that crazy "shift a constant value right by three bits, and then use
> an addressing mode to add it to another constant".

Hm, the compiler knows perfectly well how to make compound addresses,
but all this KASAN stuff is a bit special.

> And that 0xdffffc0000000000 constant is KASAN_SHADOW_OFFSET.
>
> So what I think is going on is trivial - and has nothing to do with ordering.
>
> I think gcc is simply doing a KASAN check on a percpu address.
>
> Which it shouldn't do, and didn't use to do because we did the access
> using inline asm.
>
> But now that gcc does the accesses as normal (albeit special address
> space) memory accesses, the KASAN code triggers on them too, and it
> all goes to hell in a handbasket very quickly.

Yes, I can confirm that. The failing .config from Linux Kernel Test
project works perfectly well after KASAN has been switched off.

So, the patch to fix the issue could be as simple as the one attached
to the message.

> End result: those percpu accessor functions need to disable any KASAN
> checking or other sanitizer checking. Not on the percpu address,
> because that's not a "real" address, it's obviously just the offset
> from the segment register.
>
> We have some other cases like that, see __read_once_word_nocheck().
>
> And gcc should probably not have generated such code in the first
> place, so arguably this is a bug with -fsanitize=kernel-address. How
> does gcc handle the thread pointers with address sanitizer? Does it
> convert them into real pointers first, and didn't realize that it
> can't do it for __seg_gs?

I don't know this part of the compiler well, but it should not touch
non-default namespaces. I'll file a bug report there.

Thanks,
Uros.

[-- Attachment #2: p.diff.txt --]
[-- Type: text/plain, Size: 367 bytes --]

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index ecb256954351..1edf4a5b93ca 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -2393,7 +2393,7 @@ config CC_HAS_NAMED_AS
 
 config USE_X86_SEG_SUPPORT
 	def_bool y
-	depends on CC_HAS_NAMED_AS && SMP
+	depends on CC_HAS_NAMED_AS && SMP && !KASAN
 
 config CC_HAS_SLS
 	def_bool $(cc-option,-mharden-sls=all)