linux-kernel - Re: objtool clac/stac handling change..

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:   Fri, 3 Jul 2020 14:10:08 -0700
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     Al Viro <viro@...iv.linux.org.uk>
Cc:     Michael Ellerman <mpe@...erman.id.au>,
        Christophe Leroy <christophe.leroy@....fr>,
        Josh Poimboeuf <jpoimboe@...hat.com>,
        Peter Zijlstra <peterz@...radead.org>,
        "the arch/x86 maintainers" <x86@...nel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: objtool clac/stac handling change..

On Fri, Jul 3, 2020 at 2:02 PM Al Viro <viro@...iv.linux.org.uk> wrote:
>
> Actually, for more serious problem consider arch/x86/lib/copy_user_64.S

What? No.

> In case of an unhandled fault on attempt to read an (unaligned) word,
> the damn thing falls back to this:
> SYM_CODE_START_LOCAL(.Lcopy_user_handle_tail)
>         movl %edx,%ecx
> 1:      rep movsb
> 2:      mov %ecx,%eax
>         ASM_CLAC
>         ret
>
>         _ASM_EXTABLE_UA(1b, 2b)
> SYM_CODE_END(.Lcopy_user_handle_tail)

In the case of "we did an unaligned word at the end of a page, we took
a fault, and now we have to start all over", the _least_ of our
problems is that part of "starting over" would now imply doing a
"stac" again.

Yeah, the "stac" instruction isn't hugely fast, and serializes the
pipeline, so it's a nasty 20 cycles or something.

But for chissake, this
 (a) happens approximately never
 (b) is after a fault that took a thousand cycles

so the trivial thing to do is to just say "yeah, you need to add the
STAC when your optimistic thing failed and you have to fall back to
the byte-at-a-time tail case".

It's particularly trivial since objtool would statically find all
these places, since it would warn about the ASM_CLAC without a STAC
(that's assuming Josh's patch, of course).

              Linus