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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Sun, 8 Oct 2023 23:41:39 +0200
From:   Uros Bizjak <ubizjak@...il.com>
To:     Linus Torvalds <torvalds@...ux-foundation.org>
Cc:     x86@...nel.org, linux-kernel@...r.kernel.org,
        Andy Lutomirski <luto@...nel.org>,
        Ingo Molnar <mingo@...nel.org>, Nadav Amit <namit@...are.com>,
        Brian Gerst <brgerst@...il.com>,
        Denys Vlasenko <dvlasenk@...hat.com>,
        "H . Peter Anvin" <hpa@...or.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Borislav Petkov <bp@...en8.de>,
        Josh Poimboeuf <jpoimboe@...hat.com>
Subject: Re: [PATCH 4/4] x86/percpu: Use C for percpu read/write accessors

On Sun, Oct 8, 2023 at 10:48 PM Linus Torvalds
<torvalds@...ux-foundation.org> wrote:
>
> On Sun, 8 Oct 2023 at 13:13, Linus Torvalds
> <torvalds@...ux-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.

View attachment "p.diff.txt" of type "text/plain" (367 bytes)