ACK: [SRU][Trusty][Bionic][PATCH 1/1] proc: restrict kernel stack dumps to root

[Kleber Souza]

On 12/4/18 3:21 AM, Khalid Elmously wrote:
> From: Jann Horn <jannh at google.com>
>
> CVE-2018-17972
>
> Currently, you can use /proc/self/task/*/stack to cause a stack walk on
> a task you control while it is running on another CPU.  That means that
> the stack can change under the stack walker.  The stack walker does
> have guards against going completely off the rails and into random
> kernel memory, but it can interpret random data from your kernel stack
> as instruction pointers and stack pointers.  This can cause exposure of
> kernel stack contents to userspace.
>
> Restrict the ability to inspect kernel stacks of arbitrary tasks to root
> in order to prevent a local attacker from exploiting racy stack unwinding
> to leak kernel task stack contents.  See the added comment for a longer
> rationale.
>
> There don't seem to be any users of this userspace API that can't
> gracefully bail out if reading from the file fails.  Therefore, I believe
> that this change is unlikely to break things.  In the case that this patch
> does end up needing a revert, the next-best solution might be to fake a
> single-entry stack based on wchan.
>
> Link: http://lkml.kernel.org/r/20180927153316.200286-1-jannh@google.com
> Fixes: 2ec220e27f50 ("proc: add /proc/*/stack")
> Signed-off-by: Jann Horn <jannh at google.com>
> Acked-by: Kees Cook <keescook at chromium.org>
> Cc: Alexey Dobriyan <adobriyan at gmail.com>
> Cc: Ken Chen <kenchen at google.com>
> Cc: Will Deacon <will.deacon at arm.com>
> Cc: Laura Abbott <labbott at redhat.com>
> Cc: Andy Lutomirski <luto at amacapital.net>
> Cc: Catalin Marinas <catalin.marinas at arm.com>
> Cc: Josh Poimboeuf <jpoimboe at redhat.com>
> Cc: Thomas Gleixner <tglx at linutronix.de>
> Cc: Ingo Molnar <mingo at redhat.com>
> Cc: "H . Peter Anvin" <hpa at zytor.com>
> Cc: <stable at vger.kernel.org>
> Signed-off-by: Andrew Morton <akpm at linux-foundation.org>
> Signed-off-by: Greg Kroah-Hartman <gregkh at linuxfoundation.org>
> (backported from f8a00cef17206ecd1b30d3d9f99e10d9fa707aa7)

As Juerg pointed out:

(backported from commit ...)

This can be fixed when applying the patch.

> [ kmously: Minor context adjustment ]
> Signed-off-by: Khalid Elmously <khalid.elmously at canonical.com>

Acked-by: Kleber Sacilotto de Souza <kleber.souza at canonical.com>

> ---
>  fs/proc/base.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
>
> diff --git a/fs/proc/base.c b/fs/proc/base.c
> index 4dd9d5541088..7c798e95a69e 100644
> --- a/fs/proc/base.c
> +++ b/fs/proc/base.c
> @@ -304,6 +304,20 @@ static int proc_pid_stack(struct seq_file *m, struct pid_namespace *ns,
>  	int err;
>  	int i;
>  
> +	/*
> +	 * The ability to racily run the kernel stack unwinder on a running task
> +	 * and then observe the unwinder output is scary; while it is useful for
> +	 * debugging kernel issues, it can also allow an attacker to leak kernel
> +	 * stack contents.
> +	 * Doing this in a manner that is at least safe from races would require
> +	 * some work to ensure that the remote task can not be scheduled; and
> +	 * even then, this would still expose the unwinder as local attack
> +	 * surface.
> +	 * Therefore, this interface is restricted to root.
> +	 */
> +	if (!file_ns_capable(m->file, &init_user_ns, CAP_SYS_ADMIN))
> +		return -EACCES;
> +
>  	entries = kmalloc(MAX_STACK_TRACE_DEPTH * sizeof(*entries), GFP_KERNEL);
>  	if (!entries)
>  		return -ENOMEM;