linux-kernel - Re: [PATCH v2 2/6] x86/stacktrace: make clear the success paths

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 14 May 2018 10:03:14 -0500
From:   Josh Poimboeuf <jpoimboe@...hat.com>
To:     Jiri Slaby <jslaby@...e.cz>
Cc:     mingo@...hat.com, linux-kernel@...r.kernel.org,
        Thomas Gleixner <tglx@...utronix.de>,
        "H. Peter Anvin" <hpa@...or.com>, x86@...nel.org
Subject: Re: [PATCH v2 2/6] x86/stacktrace: make clear the success paths

On Mon, May 14, 2018 at 04:06:49PM +0200, Jiri Slaby wrote:
> Make clear which path is for user tasks and for kthreads and idle
> tasks. This will allow easier plug-in of ORC unwinder in the next
> patches.
> 
> Signed-off-by: Jiri Slaby <jslaby@...e.cz>
> Cc: Thomas Gleixner <tglx@...utronix.de>
> Cc: Ingo Molnar <mingo@...hat.com>
> Cc: "H. Peter Anvin" <hpa@...or.com>
> Cc: x86@...nel.org
> Cc: Josh Poimboeuf <jpoimboe@...hat.com>
> ---
>  arch/x86/kernel/stacktrace.c | 13 ++++++++++---
>  1 file changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/x86/kernel/stacktrace.c b/arch/x86/kernel/stacktrace.c
> index 8948b7d9c064..3a4602c2324f 100644
> --- a/arch/x86/kernel/stacktrace.c
> +++ b/arch/x86/kernel/stacktrace.c
> @@ -104,16 +104,18 @@ __save_stack_trace_reliable(struct stack_trace *trace,
>  
>  		regs = unwind_get_entry_regs(&state, NULL);
>  		if (regs) {
> +			/* Success path for user tasks */
> +			if (user_mode(regs))
> +				goto success;
> +
>  			/*
>  			 * Kernel mode registers on the stack indicate an
>  			 * in-kernel interrupt or exception (e.g., preemption
>  			 * or a page fault), which can make frame pointers
>  			 * unreliable.
>  			 */
> -			if (!user_mode(regs))
> -				return -EINVAL;
>  
> -			break;
> +			return -EINVAL;
>  		}
>  
>  		addr = unwind_get_return_address(&state);
> @@ -138,6 +140,11 @@ __save_stack_trace_reliable(struct stack_trace *trace,
>  		return -EINVAL;
>  	}
>  
> +	/* Success path for non-user tasks, i.e. kthreads and idle tasks */
> +	if (!(task->flags & (PF_KTHREAD | PF_IDLE)))
> +		return -EINVAL;
> +
> +success:
>  	if (trace->nr_entries < trace->max_entries)
>  		trace->entries[trace->nr_entries++] = ULONG_MAX;
>  
> -- 
> 2.16.3
> 

The unwind_error() check is now getting skipped on the user mode
success path.  It would probably be safer to check it at the very top of
the loop so that it always gets checked.

Also the subject can be "clarified" a bit:

  x86/stacktrace: Clarify the reliable success paths

-- 
Josh