linux-api.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Kees Cook <keescook-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
To: Thomas Garnier <thgarnie-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>,
	Ingo Molnar <mingo-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Cc: "Martin Schwidefsky"
	<schwidefsky-tA70FqPdS9bQT0dZR+AlfA@public.gmane.org>,
	"Heiko Carstens"
	<heiko.carstens-tA70FqPdS9bQT0dZR+AlfA@public.gmane.org>,
	"Arnd Bergmann" <arnd-r2nGTMty4D4@public.gmane.org>,
	"Dave Hansen"
	<dave.hansen-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>,
	"Andrew Morton"
	<akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>,
	"David Howells"
	<dhowells-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>,
	"René Nyffenegger"
	<mail-gLCNRsNSrVdVZEhyV+6z5nIPMjoJpjVV@public.gmane.org>,
	"Paul E . McKenney"
	<paulmck-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>,
	"Thomas Gleixner" <tglx-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org>,
	"Oleg Nesterov" <oleg-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>,
	"Stephen Smalley" <sds-+05T5uksL2qpZYMLLGbcSA@public.gmane.org>,
	"Pavel Tikhomirov"
	<ptikhomirov-5HdwGun5lf+gSpxsJD1C4w@public.gmane.org>,
	"Ingo Molnar" <mingo-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>,
	"H . Peter Anvin" <hpa-YMNOUZJC4hwAvxtiuMwx3w@public.gmane.org>,
	"Andy Lutomirski" <luto-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>,
	"Paolo Bonzini"
	<pbonzini-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>,
	"Rik van Riel" <riel-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>,
	"Josh Poimboeuf"
	<jpoimboe-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>,
	"Borislav Petkov" <bp-Gina5bIWoIWzQB+pC5nmwQ@public.gmane.org>
Subject: Re: [PATCH v7 1/4] syscalls: Restore address limit after a syscall
Date: Mon, 24 Apr 2017 16:57:34 -0700	[thread overview]
Message-ID: <CAGXu5j+J67zC9tcncacHTDvXKMF3HZuPCivk-Uz4J6h-cEv-qQ@mail.gmail.com> (raw)
In-Reply-To: <20170410164420.64003-1-thgarnie-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>

On Mon, Apr 10, 2017 at 9:44 AM, Thomas Garnier <thgarnie-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org> wrote:
> This patch ensures a syscall does not return to user-mode with a kernel
> address limit. If that happened, a process can corrupt kernel-mode
> memory and elevate privileges.
>
> For example, it would mitigation this bug:
>
> - https://bugs.chromium.org/p/project-zero/issues/detail?id=990
>
> The CONFIG_ARCH_NO_SYSCALL_VERIFY_PRE_USERMODE_STATE option is also
> added so each architecture can optimize this change.
>
> Signed-off-by: Thomas Garnier <thgarnie-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
> Tested-by: Kees Cook <keescook-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>

Ingo, I think this series is ready. Can you pull it? (And if not, what
should next steps be?)

-Kees

> ---
> Based on next-20170410
> ---
>  arch/s390/Kconfig        |  1 +
>  include/linux/syscalls.h | 26 +++++++++++++++++++++++++-
>  init/Kconfig             |  6 ++++++
>  kernel/sys.c             | 13 +++++++++++++
>  4 files changed, 45 insertions(+), 1 deletion(-)
>
> diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
> index d25435d94b6e..489a0cc6e46b 100644
> --- a/arch/s390/Kconfig
> +++ b/arch/s390/Kconfig
> @@ -103,6 +103,7 @@ config S390
>         select ARCH_INLINE_WRITE_UNLOCK_BH
>         select ARCH_INLINE_WRITE_UNLOCK_IRQ
>         select ARCH_INLINE_WRITE_UNLOCK_IRQRESTORE
> +       select ARCH_NO_SYSCALL_VERIFY_PRE_USERMODE_STATE
>         select ARCH_SAVE_PAGE_KEYS if HIBERNATION
>         select ARCH_SUPPORTS_ATOMIC_RMW
>         select ARCH_SUPPORTS_DEFERRED_STRUCT_PAGE_INIT
> diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
> index 980c3c9b06f8..801a7a74fe28 100644
> --- a/include/linux/syscalls.h
> +++ b/include/linux/syscalls.h
> @@ -191,6 +191,27 @@ extern struct trace_event_functions exit_syscall_print_funcs;
>         SYSCALL_METADATA(sname, x, __VA_ARGS__)                 \
>         __SYSCALL_DEFINEx(x, sname, __VA_ARGS__)
>
> +
> +/*
> + * Called before coming back to user-mode. Returning to user-mode with an
> + * address limit different than USER_DS can allow to overwrite kernel memory.
> + */
> +static inline void verify_pre_usermode_state(void) {
> +       BUG_ON(!segment_eq(get_fs(), USER_DS));
> +}
> +
> +#ifndef CONFIG_ARCH_NO_SYSCALL_VERIFY_PRE_USERMODE_STATE
> +#define __CHECK_USER_CALLER() \
> +       bool user_caller = segment_eq(get_fs(), USER_DS)
> +#define __VERIFY_PRE_USERMODE_STATE() \
> +       if (user_caller) verify_pre_usermode_state()
> +#else
> +#define __CHECK_USER_CALLER()
> +#define __VERIFY_PRE_USERMODE_STATE()
> +asmlinkage void address_limit_check_failed(void);
> +#endif
> +
> +
>  #define __PROTECT(...) asmlinkage_protect(__VA_ARGS__)
>  #define __SYSCALL_DEFINEx(x, name, ...)                                        \
>         asmlinkage long sys##name(__MAP(x,__SC_DECL,__VA_ARGS__))       \
> @@ -199,7 +220,10 @@ extern struct trace_event_functions exit_syscall_print_funcs;
>         asmlinkage long SyS##name(__MAP(x,__SC_LONG,__VA_ARGS__));      \
>         asmlinkage long SyS##name(__MAP(x,__SC_LONG,__VA_ARGS__))       \
>         {                                                               \
> -               long ret = SYSC##name(__MAP(x,__SC_CAST,__VA_ARGS__));  \
> +               long ret;                                               \
> +               __CHECK_USER_CALLER();                                  \
> +               ret = SYSC##name(__MAP(x,__SC_CAST,__VA_ARGS__));       \
> +               __VERIFY_PRE_USERMODE_STATE();                          \
>                 __MAP(x,__SC_TEST,__VA_ARGS__);                         \
>                 __PROTECT(x, ret,__MAP(x,__SC_ARGS,__VA_ARGS__));       \
>                 return ret;                                             \
> diff --git a/init/Kconfig b/init/Kconfig
> index 7f7027817bce..e5fbd0becfa7 100644
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -1958,6 +1958,12 @@ config PROFILING
>  config TRACEPOINTS
>         bool
>
> +config ARCH_NO_SYSCALL_VERIFY_PRE_USERMODE_STATE
> +       bool
> +       help
> +         Disable the generic pre-usermode state verification. Allow each
> +         architecture to optimize how and when the verification is done.
> +
>  source "arch/Kconfig"
>
>  endmenu                # General setup
> diff --git a/kernel/sys.c b/kernel/sys.c
> index 196c7134bee6..d30530ff8166 100644
> --- a/kernel/sys.c
> +++ b/kernel/sys.c
> @@ -2459,3 +2459,16 @@ COMPAT_SYSCALL_DEFINE1(sysinfo, struct compat_sysinfo __user *, info)
>         return 0;
>  }
>  #endif /* CONFIG_COMPAT */
> +
> +#ifdef CONFIG_ARCH_NO_SYSCALL_VERIFY_PRE_USERMODE_STATE
> +/*
> + * This function is called when an architecture specific implementation detected
> + * an invalid address limit. The generic user-mode state checker will finish on
> + * the appropriate BUG_ON.
> + */
> +asmlinkage void address_limit_check_failed(void)
> +{
> +       verify_pre_usermode_state();
> +       panic("address_limit_check_failed called with a valid user-mode state");
> +}
> +#endif
> --
> 2.12.2.715.g7642488e1d-goog
>



-- 
Kees Cook
Pixel Security

  parent reply	other threads:[~2017-04-24 23:57 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-04-10 16:44 [PATCH v7 1/4] syscalls: Restore address limit after a syscall Thomas Garnier
2017-04-10 16:44 ` [PATCH v7 3/4] arm/syscalls: Architecture specific pre-usermode check Thomas Garnier
2017-04-10 16:44 ` [PATCH v7 4/4] arm64/syscalls: " Thomas Garnier
2017-04-10 17:12   ` Catalin Marinas
     [not found]     ` <20170410171240.GE30647-M2fw3Uu6cmfZROr8t4l/smS4ubULX0JqMm0uRHvK7Nw@public.gmane.org>
2017-04-10 20:06       ` Thomas Garnier
     [not found]         ` <20170410200622.8654-1-thgarnie-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
2017-04-10 20:09           ` Thomas Garnier
2017-04-10 20:07       ` Thomas Garnier
     [not found] ` <20170410164420.64003-1-thgarnie-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
2017-04-10 16:44   ` [PATCH v7 2/4] x86/syscalls: " Thomas Garnier
2017-04-24 23:57   ` Kees Cook [this message]
     [not found]     ` <CAGXu5j+J67zC9tcncacHTDvXKMF3HZuPCivk-Uz4J6h-cEv-qQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2017-04-25  6:23       ` [PATCH v7 1/4] syscalls: Restore address limit after a syscall Ingo Molnar
     [not found]         ` <20170425062324.pdpi5v7ypobw74ki-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2017-04-25 14:12           ` Thomas Garnier
2017-04-25  6:33   ` Ingo Molnar
     [not found]     ` <20170425063305.hwjuxupa37rwe6zj-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2017-04-25 14:18       ` Thomas Garnier
2017-04-26  8:12         ` Ingo Molnar
     [not found]           ` <20170426081229.6wnugrs7w3at4xry-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2017-04-26 14:09             ` Thomas Garnier

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAGXu5j+J67zC9tcncacHTDvXKMF3HZuPCivk-Uz4J6h-cEv-qQ@mail.gmail.com \
    --to=keescook-f7+t8e8rja9g9huczpvpmw@public.gmane.org \
    --cc=akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org \
    --cc=arnd-r2nGTMty4D4@public.gmane.org \
    --cc=bp-Gina5bIWoIWzQB+pC5nmwQ@public.gmane.org \
    --cc=dave.hansen-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org \
    --cc=dhowells-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org \
    --cc=heiko.carstens-tA70FqPdS9bQT0dZR+AlfA@public.gmane.org \
    --cc=hpa-YMNOUZJC4hwAvxtiuMwx3w@public.gmane.org \
    --cc=jpoimboe-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org \
    --cc=luto-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org \
    --cc=mail-gLCNRsNSrVdVZEhyV+6z5nIPMjoJpjVV@public.gmane.org \
    --cc=mingo-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org \
    --cc=mingo-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org \
    --cc=oleg-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org \
    --cc=paulmck-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org \
    --cc=pbonzini-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org \
    --cc=ptikhomirov-5HdwGun5lf+gSpxsJD1C4w@public.gmane.org \
    --cc=riel-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org \
    --cc=schwidefsky-tA70FqPdS9bQT0dZR+AlfA@public.gmane.org \
    --cc=sds-+05T5uksL2qpZYMLLGbcSA@public.gmane.org \
    --cc=tglx-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org \
    --cc=thgarnie-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).