linux-kernel - Re: [PATCH] ARC: uaccess: get_user to zero out dest in cause of fault

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, 19 Aug 2016 15:00:47 -0700
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     Al Viro <viro@...iv.linux.org.uk>
Cc:     Vineet Gupta <Vineet.Gupta1@...opsys.com>,
        "linux-arch@...r.kernel.org" <linux-arch@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        "H. Peter Anvin" <hpa@...or.com>, Ingo Molnar <mingo@...hat.com>
Subject: Re: [PATCH] ARC: uaccess: get_user to zero out dest in cause of fault

On Fri, Aug 19, 2016 at 2:24 PM, Al Viro <viro@...iv.linux.org.uk> wrote:
>         * x86-only get_user_ex().  Does *not* return anything, uses per-process
> flag to indicate errors, the entire sequence is bracketed by uaccess_try()
> and uaccess_catch(err), the latter dumps the flag into err.

I really don't want people to copy that pattern.

It's absolutely horrendous if you start taking faults, and you'll take
fault after fault after fault - in kernel space too. Yes, faults are
very very rare, but still, the interface is bad.

I'd much rather other architectures used the "unsafe_get_user()"
model, which currently only "strncpy_from_user()" and friends use. If
gcc ever ends up supporting "asm goto" with outputs, that interface is
actually able to generate pretty much optimal code.

And "unsafe_put_user()" can actually generate "perfect" code *today*,
because it doesn't have outputs, so "asm goto" actually works right
now. You can make it do the branch-out directly from the exception
case. It's not used right now, but as a replacement for the nasty
"put_user_ex()" model, it's actually much much better.

(I have some experimental patches that actually use "asm goto" in
"unsafe_put_user()" to get that nice code generation, but they only
work if your gcc version supports "asm goto", which some older
versions of gcc does not)

> Suggestions?

Please see "unsafe_put_user(x, ptr, error_label)" as the future. No,
right now it ends up doing the same old thing, but that is _fixable_
unlike the other strange special cases.

Side note: the "error-label" form was introduced in this merge window,
exactly because I wanted to have an interface that is optimizable in
the future.

See commit 1bd4403d86a1 ("unsafe_[get|put]_user: change interface to
use a error target label")

            Linus