linux-kernel - Re: [RFC PATCH 0/4] Allow tasks to have their user stack pointer sanity checked

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] [day] [month] [year] [list]

Message-ID: <20190213131915.GC5875@brain-police>
Date:   Wed, 13 Feb 2019 13:19:16 +0000
From:   Will Deacon <will.deacon@....com>
To:     Kees Cook <keescook@...omium.org>
Cc:     LKML <linux-kernel@...r.kernel.org>, Jann Horn <jannh@...gle.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Matthew Wilcox <willy@...radead.org>,
        Michal Hocko <mhocko@...e.com>,
        Peter Zijlstra <peterz@...radead.org>
Subject: Re: [RFC PATCH 0/4] Allow tasks to have their user stack pointer
 sanity checked

Hi Kees,

On Mon, Feb 11, 2019 at 11:12:19AM -0800, Kees Cook wrote:
> On Mon, Feb 11, 2019 at 9:59 AM Will Deacon <will.deacon@....com> wrote:
> > I attended an interesting talk at LCA last month that described some of the
> > security features deployed in OpenBSD [1]. One hardening feature that piqued
> > my interest was, on syscall entry and page faults from userspace, checking
> > that the user stack pointer for a task points at pages that were either
> > allocated by the kernel for the initial process stack of mapped with mmap()
> > using the MAP_STACK flag. This acts as a basic defense against stack
> > pivoting attacks.
> 
> I think this is nice to have, yes! Thanks for working on it. It seems
> like this blocks pivots to heap -- relocating to a groomed stack area
> would still be allowed. Regardless, this does narrow the scope of such
> attacks quite nicely.
> 
> > The problem with this checking is that it is a retrospective tightening
> > of the ABI, but that hasn't stopped me hacking it together behind a couple
> > of prctl() options.
> 
> MAP_STACK has been around for a long time, so I think anything using
> threads via glibc should be "covered". I would assume this would mean
> that glibc could set the prctl() for such users. I suspect there are a
> lot of open-coded threading implementations, though. It'd be
> interesting to see how many need modification.
> 
> Given that this is behind a prctl(), it seems the CONFIG isn't needed?

I wanted to keep the CONFIG because we grow task_struct and maybe somebody
cares about that (many of the other fields in there are guarded).

> > Anyway, it was fun to implement so I figured I'd post it as an RFC.
> 
> Thanks! I'd love to see an x86 counterpart to the sycall check too.

I'll take a quick look. I think that, like arm64, x86 moved much of their
entry code into C so it might be really straightforward.

> Did you trying bringing up a full userspace and windowing environment
> with this enabled by default (i.e. forcing init to set the prctls)?
> I'd be curious to see how much (if anything) goes boom. :)

So far I haven't found anything other than my targetted testcase which
explodes. However, I would fully expect some JITs to go wrong and probably
also some uses of sigaltstack().

> Reviewed-by: Kees Cook <keescook@...omium.org>

Thanks!

Will