[RFC,0/4] Allow tasks to have their user stack pointer sanity checked

Message ID 20190211175935.4602-1-will.deacon@arm.com
Headers show
Series
  • Allow tasks to have their user stack pointer sanity checked
Related show

Message

Will Deacon Feb. 11, 2019, 5:59 p.m.
Hi all,

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.

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.

Anyway, it was fun to implement so I figured I'd post it as an RFC.

Will

[1] https://2019.linux.conf.au/schedule/presentation/164/

Cc: Kees Cook <keescook@chromium.org>
Cc: Jann Horn <jannh@google.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Matthew Wilcox <willy@infradead.org>
Cc: Michal Hocko <mhocko@suse.com>
Cc: Peter Zijlstra <peterz@infradead.org>

--->8

Will Deacon (4):
  mm: Check user stack pointer is mapped with MAP_STACK
  mm: Expose user stack pointer checking via prctl()
  mm: Add kconfig entries for user stack pointer checking
  arm64: Check user stack pointer on syscall entry

 arch/arm64/Kconfig          |  1 +
 arch/arm64/kernel/syscall.c |  4 +++
 include/linux/mm.h          | 15 +++++++++-
 include/linux/mman.h        |  3 +-
 include/linux/sched.h       |  4 +++
 include/uapi/linux/prctl.h  |  5 ++++
 kernel/sys.c                |  5 ++++
 mm/Kconfig                  | 17 ++++++++++++
 mm/memory.c                 | 67 +++++++++++++++++++++++++++++++++++++++++++++
 9 files changed, 119 insertions(+), 2 deletions(-)

-- 
2.11.0

Comments

Kees Cook Feb. 11, 2019, 7:12 p.m. | #1
On Mon, Feb 11, 2019 at 9:59 AM Will Deacon <will.deacon@arm.com> wrote:
>

> Hi all,

>

> 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?

> 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.

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. :)

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


-Kees

>

> Will

>

> [1] https://2019.linux.conf.au/schedule/presentation/164/

>

> Cc: Kees Cook <keescook@chromium.org>

> Cc: Jann Horn <jannh@google.com>

> Cc: Andrew Morton <akpm@linux-foundation.org>

> Cc: Matthew Wilcox <willy@infradead.org>

> Cc: Michal Hocko <mhocko@suse.com>

> Cc: Peter Zijlstra <peterz@infradead.org>

>

> --->8

>

> Will Deacon (4):

>   mm: Check user stack pointer is mapped with MAP_STACK

>   mm: Expose user stack pointer checking via prctl()

>   mm: Add kconfig entries for user stack pointer checking

>   arm64: Check user stack pointer on syscall entry

>

>  arch/arm64/Kconfig          |  1 +

>  arch/arm64/kernel/syscall.c |  4 +++

>  include/linux/mm.h          | 15 +++++++++-

>  include/linux/mman.h        |  3 +-

>  include/linux/sched.h       |  4 +++

>  include/uapi/linux/prctl.h  |  5 ++++

>  kernel/sys.c                |  5 ++++

>  mm/Kconfig                  | 17 ++++++++++++

>  mm/memory.c                 | 67 +++++++++++++++++++++++++++++++++++++++++++++

>  9 files changed, 119 insertions(+), 2 deletions(-)

>

> --

> 2.11.0

>



-- 
Kees Cook
Will Deacon Feb. 13, 2019, 1:19 p.m. | #2
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@arm.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@chromium.org>


Thanks!

Will