linux-kernel - Re: [PATCH v4 54/57] x86/mm: convert arch_within_stack

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:   Mon, 22 Aug 2016 15:11:32 -0700
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     Josh Poimboeuf <jpoimboe@...hat.com>
Cc:     Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...nel.org>,
        "H . Peter Anvin" <hpa@...or.com>,
        "the arch/x86 maintainers" <x86@...nel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Andy Lutomirski <luto@...capital.net>,
        Steven Rostedt <rostedt@...dmis.org>,
        Brian Gerst <brgerst@...il.com>,
        Kees Cook <keescook@...omium.org>,
        Peter Zijlstra <peterz@...radead.org>,
        Frederic Weisbecker <fweisbec@...il.com>,
        Byungchul Park <byungchul.park@....com>,
        Nilay Vaish <nilayvaish@...il.com>
Subject: Re: [PATCH v4 54/57] x86/mm: convert arch_within_stack_frames() to
 use the new unwinder

On Thu, Aug 18, 2016 at 6:06 AM, Josh Poimboeuf <jpoimboe@...hat.com> wrote:
> Convert arch_within_stack_frames() to use the new unwinder.

Please don't do this.

There's no real reason to unwind the stack frame. If it's not on the
current stack page, it shouldn't be a valid source anyway, so
unwidning things just seems entirely pointless.

Quite frankly, I think the whole "look at the stack frames" logic
should be removed from this. It's classic crap that external patches
do. How many call-sites does it actually check, and how many of them
aren't already checked by the existing static checks for constant
addresses within existing objects?

It's entirely possible that there is simply no point what-so-ever to
this all, and it mostly triggers on things like the fs/stat.c code
that does

        struct stat tmp;
    ...
        return copy_to_user(statbuf,&tmp,sizeof(tmp)) ? -EFAULT : 0;

where the new useraccess.c code is pure masturbatory crap.

One of the reasons I had for merging that code was that I was hoping
that it would improve by being in the  kernel. And by "improve" I mean
"get rid of crap" rather than make it more expensive and even more
self-congratulatory stupidity.

Right now, I suspect 99% of all the stack checks in usercopy.c are
solidly in the "mindbogglingly stupid crap" camp.

             Linus