linux-kernel - Re: [RFC][CFT][PATCHSET v1] uaccess unification

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: <CA+55aFzsGSKzbQjWAPxFvc=HeXyr6stk1KsB-1xGCKALgazYuQ@mail.gmail.com>
Date:   Thu, 30 Mar 2017 10:18:23 -0700
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     Al Viro <viro@...iv.linux.org.uk>
Cc:     Russell King - ARM Linux <linux@...linux.org.uk>,
        "linux-arch@...r.kernel.org" <linux-arch@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Richard Henderson <rth@...ddle.net>,
        Will Deacon <will.deacon@....com>,
        Haavard Skinnemoen <hskinnemoen@...il.com>,
        Vineet Gupta <vgupta@...opsys.com>,
        Steven Miao <realmz6@...il.com>,
        Jesper Nilsson <jesper.nilsson@...s.com>,
        Mark Salter <msalter@...hat.com>,
        Yoshinori Sato <ysato@...rs.sourceforge.jp>,
        Richard Kuo <rkuo@...eaurora.org>,
        Tony Luck <tony.luck@...el.com>,
        Geert Uytterhoeven <geert@...ux-m68k.org>,
        James Hogan <james.hogan@...tec.com>,
        Michal Simek <monstr@...str.eu>,
        David Howells <dhowells@...hat.com>,
        Ley Foon Tan <lftan@...era.com>,
        Jonas Bonn <jonas@...thpole.se>, Helge Deller <deller@....de>,
        Martin Schwidefsky <schwidefsky@...ibm.com>,
        Ralf Baechle <ralf@...ux-mips.org>,
        Benjamin Herrenschmidt <benh@...nel.crashing.org>,
        Chen Liqin <liqin.linux@...il.com>,
        "David S. Miller" <davem@...emloft.net>,
        Chris Metcalf <cmetcalf@...lanox.com>,
        Richard Weinberger <richard@....at>,
        Guan Xuetao <gxt@...c.pku.edu.cn>,
        Thomas Gleixner <tglx@...utronix.de>,
        Chris Zankel <chris@...kel.net>
Subject: Re: [RFC][CFT][PATCHSET v1] uaccess unification

On Thu, Mar 30, 2017 at 9:43 AM, Al Viro <viro@...iv.linux.org.uk> wrote:
> On Thu, Mar 30, 2017 at 05:22:41PM +0100, Russell King - ARM Linux wrote:
> How would the following affect things?
>
> diff --git a/lib/iov_iter.c b/lib/iov_iter.c
> index e68604ae3ced..d24d338f0682 100644
> --- a/lib/iov_iter.c
> +++ b/lib/iov_iter.c
> @@ -184,7 +184,7 @@ static size_t copy_page_to_iter_iovec(struct page *page, size_t offset, size_t b
>
>         kaddr = kmap(page);
>         from = kaddr + offset;
> -       left = __copy_to_user(buf, from, copy);
> +       left = __copy_to_user_inatomic(buf, from, copy);

This is all going in the wrong direction entirely.

That "__copy_to_user()" code was bad from the beginning: it should
never have had the double underscores. I objected to it at the time.

Now you're making it go from bad to insane. You're apparently
mis-using "inatomic" because of subtle issues that have nothing to do
with "inatomic" - you want to get rid of a might_sleep() warning, but
you don't actuially want inatomic behavior, so the thing will still
sleep.

This all very subtle already depends on people having checked the
"struct iov_iter" beforehand. We should *remove* subtle stuff like
that, not add yet more layers of subtlety and possible future bugs
when somebody calls copy_page_to_iter() without having properly
validated the iter.

These are not theoretical issues. We've _had_ these exact bugs when
people didn't validate the stuff they created by hand and bypassed the
normal IO paths.

Trying to optimize away an access_ok() or a might_fault() is *not* a
valid reason to completely break our security model, and create code
that makes no sense (claiming it is atomic when it isn't).

                  Linus