| Message ID | 20190211175935.4602-1-will.deacon@arm.com |
|---|---|
| Headers | show |
| Series | Allow tasks to have their user stack pointer sanity checked | expand |
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
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