linux-kernel - Re: [PATCH] x86: only use ERMS for user copies for larger sizes

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]

Message-ID: <CAHk-=wiDLq5CVnTJhFNEEduqYP_VSsQPSNvi=w6a8Y9figuh7g@mail.gmail.com>
Date:   Wed, 21 Nov 2018 11:01:35 -0800
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     pabeni@...hat.com
Cc:     Jens Axboe <axboe@...nel.dk>, Ingo Molnar <mingo@...nel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, bp@...en8.de,
        Peter Anvin <hpa@...or.com>,
        "the arch/x86 maintainers" <x86@...nel.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Andrew Lutomirski <luto@...nel.org>,
        Peter Zijlstra <a.p.zijlstra@...llo.nl>, dvlasenk@...hat.com,
        brgerst@...il.com,
        Linux List Kernel Mailing <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] x86: only use ERMS for user copies for larger sizes

On Wed, Nov 21, 2018 at 10:16 AM Linus Torvalds
<torvalds@...ux-foundation.org> wrote:
>
> It might be interesting to just change raw_copy_to/from_user() to
> handle a lot more cases (in particular, handle cases where 'size' is
> 8-byte aligned). The special cases we *do* have may not be the right
> ones (the 10-byte case in particular looks odd).
>
> For example, instead of having a "if constant size is 8 bytes, do one
> get/put_user()" case, we might have a "if constant size is < 64 just
> unroll it into get/put_user()" calls.

Actually, x86 doesn't even set INLINE_COPY_TO_USER, so I don't think
the constant size cases ever trigger at all the way they are set up
now.

I do have a random patch that makes "unsafe_put_user()" actually use
"asm goto" for the error case, and that, together with the attached
patch seems to generate fairly nice code, but even then it would
depend on gcc actually unrolling things (which we do *not* want in
general).

But for a 32-byte user copy (cp_old_stat), and that
INLINE_COPY_TO_USER, it generates this:

        stac
        movl    $32, %edx       #, size
        movq    %rsp, %rax      #, src
.L201:
        movq    (%rax), %rcx    # MEM[base: src_155, offset: 0B],
MEM[base: src_155, offset: 0B]
1:      movq %rcx,0(%rbp)       # MEM[base: src_155, offset: 0B],
MEM[(struct __large_struct *)dst_156]
ASM_EXTABLE_HANDLE from=1b to=.L200 handler="ex_handler_uaccess"        #

        addq    $8, %rax        #, src
        addq    $8, %rbp        #, statbuf
        subq    $8, %rdx        #, size
        jne     .L201   #,
        clac

which is actually fairly close to "optimal".

Random patch (with my "asm goto" hack included) attached, in case
people want to play with it.

Impressively, it actually removes more lines of code than it adds. But
I didn't actually check whether the end result *works*, so hey..

                  Linus

View attachment "patch.diff" of type "text/x-patch" (13387 bytes)