mirror of
				https://github.com/torvalds/linux.git
				synced 2025-11-04 10:40:15 +02:00 
			
		
		
		
	x86/dumpstack: Fix partial register dumps
The show_regs_safe() logic is wrong. When there's an iret stack frame, it prints the entire pt_regs -- most of which is random stack data -- instead of just the five registers at the end. show_regs_safe() is also poorly named: the on_stack() checks aren't for safety. Rename the function to show_regs_if_on_stack() and add a comment to explain why the checks are needed. These issues were introduced with the "partial register dump" feature of the following commit:b02fcf9ba1("x86/unwinder: Handle stack overflows more gracefully") That patch had gone through a few iterations of development, and the above issues were artifacts from a previous iteration of the patch where 'regs' pointed directly to the iret frame rather than to the (partially empty) pt_regs. Tested-by: Alexander Tsoy <alexander@tsoy.me> Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com> Cc: Andy Lutomirski <luto@kernel.org> Cc: Linus Torvalds <torvalds@linux-foundation.org> Cc: Peter Zijlstra <peterz@infradead.org> Cc: Thomas Gleixner <tglx@linutronix.de> Cc: Toralf Förster <toralf.foerster@gmx.de> Cc: stable@vger.kernel.org Fixes:b02fcf9ba1("x86/unwinder: Handle stack overflows more gracefully") Link: http://lkml.kernel.org/r/5b05b8b344f59db2d3d50dbdeba92d60f2304c54.1514736742.git.jpoimboe@redhat.com Signed-off-by: Ingo Molnar <mingo@kernel.org>
This commit is contained in:
		
							parent
							
								
									52994c256d
								
							
						
					
					
						commit
						a9cdbe72c4
					
				
					 3 changed files with 34 additions and 13 deletions
				
			
		| 
						 | 
					@ -56,18 +56,27 @@ void unwind_start(struct unwind_state *state, struct task_struct *task,
 | 
				
			||||||
 | 
					
 | 
				
			||||||
#if defined(CONFIG_UNWINDER_ORC) || defined(CONFIG_UNWINDER_FRAME_POINTER)
 | 
					#if defined(CONFIG_UNWINDER_ORC) || defined(CONFIG_UNWINDER_FRAME_POINTER)
 | 
				
			||||||
/*
 | 
					/*
 | 
				
			||||||
 * WARNING: The entire pt_regs may not be safe to dereference.  In some cases,
 | 
					 * If 'partial' returns true, only the iret frame registers are valid.
 | 
				
			||||||
 * only the iret frame registers are accessible.  Use with caution!
 | 
					 | 
				
			||||||
 */
 | 
					 */
 | 
				
			||||||
static inline struct pt_regs *unwind_get_entry_regs(struct unwind_state *state)
 | 
					static inline struct pt_regs *unwind_get_entry_regs(struct unwind_state *state,
 | 
				
			||||||
 | 
											    bool *partial)
 | 
				
			||||||
{
 | 
					{
 | 
				
			||||||
	if (unwind_done(state))
 | 
						if (unwind_done(state))
 | 
				
			||||||
		return NULL;
 | 
							return NULL;
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
						if (partial) {
 | 
				
			||||||
 | 
					#ifdef CONFIG_UNWINDER_ORC
 | 
				
			||||||
 | 
							*partial = !state->full_regs;
 | 
				
			||||||
 | 
					#else
 | 
				
			||||||
 | 
							*partial = false;
 | 
				
			||||||
 | 
					#endif
 | 
				
			||||||
 | 
						}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
	return state->regs;
 | 
						return state->regs;
 | 
				
			||||||
}
 | 
					}
 | 
				
			||||||
#else
 | 
					#else
 | 
				
			||||||
static inline struct pt_regs *unwind_get_entry_regs(struct unwind_state *state)
 | 
					static inline struct pt_regs *unwind_get_entry_regs(struct unwind_state *state,
 | 
				
			||||||
 | 
											    bool *partial)
 | 
				
			||||||
{
 | 
					{
 | 
				
			||||||
	return NULL;
 | 
						return NULL;
 | 
				
			||||||
}
 | 
					}
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
| 
						 | 
					@ -76,12 +76,23 @@ void show_iret_regs(struct pt_regs *regs)
 | 
				
			||||||
		regs->sp, regs->flags);
 | 
							regs->sp, regs->flags);
 | 
				
			||||||
}
 | 
					}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
static void show_regs_safe(struct stack_info *info, struct pt_regs *regs)
 | 
					static void show_regs_if_on_stack(struct stack_info *info, struct pt_regs *regs,
 | 
				
			||||||
 | 
									  bool partial)
 | 
				
			||||||
{
 | 
					{
 | 
				
			||||||
	if (on_stack(info, regs, sizeof(*regs)))
 | 
						/*
 | 
				
			||||||
 | 
						 * These on_stack() checks aren't strictly necessary: the unwind code
 | 
				
			||||||
 | 
						 * has already validated the 'regs' pointer.  The checks are done for
 | 
				
			||||||
 | 
						 * ordering reasons: if the registers are on the next stack, we don't
 | 
				
			||||||
 | 
						 * want to print them out yet.  Otherwise they'll be shown as part of
 | 
				
			||||||
 | 
						 * the wrong stack.  Later, when show_trace_log_lvl() switches to the
 | 
				
			||||||
 | 
						 * next stack, this function will be called again with the same regs so
 | 
				
			||||||
 | 
						 * they can be printed in the right context.
 | 
				
			||||||
 | 
						 */
 | 
				
			||||||
 | 
						if (!partial && on_stack(info, regs, sizeof(*regs))) {
 | 
				
			||||||
		__show_regs(regs, 0);
 | 
							__show_regs(regs, 0);
 | 
				
			||||||
	else if (on_stack(info, (void *)regs + IRET_FRAME_OFFSET,
 | 
					
 | 
				
			||||||
			  IRET_FRAME_SIZE)) {
 | 
						} else if (partial && on_stack(info, (void *)regs + IRET_FRAME_OFFSET,
 | 
				
			||||||
 | 
									       IRET_FRAME_SIZE)) {
 | 
				
			||||||
		/*
 | 
							/*
 | 
				
			||||||
		 * When an interrupt or exception occurs in entry code, the
 | 
							 * When an interrupt or exception occurs in entry code, the
 | 
				
			||||||
		 * full pt_regs might not have been saved yet.  In that case
 | 
							 * full pt_regs might not have been saved yet.  In that case
 | 
				
			||||||
| 
						 | 
					@ -98,6 +109,7 @@ void show_trace_log_lvl(struct task_struct *task, struct pt_regs *regs,
 | 
				
			||||||
	struct stack_info stack_info = {0};
 | 
						struct stack_info stack_info = {0};
 | 
				
			||||||
	unsigned long visit_mask = 0;
 | 
						unsigned long visit_mask = 0;
 | 
				
			||||||
	int graph_idx = 0;
 | 
						int graph_idx = 0;
 | 
				
			||||||
 | 
						bool partial;
 | 
				
			||||||
 | 
					
 | 
				
			||||||
	printk("%sCall Trace:\n", log_lvl);
 | 
						printk("%sCall Trace:\n", log_lvl);
 | 
				
			||||||
 | 
					
 | 
				
			||||||
| 
						 | 
					@ -140,7 +152,7 @@ void show_trace_log_lvl(struct task_struct *task, struct pt_regs *regs,
 | 
				
			||||||
			printk("%s <%s>\n", log_lvl, stack_name);
 | 
								printk("%s <%s>\n", log_lvl, stack_name);
 | 
				
			||||||
 | 
					
 | 
				
			||||||
		if (regs)
 | 
							if (regs)
 | 
				
			||||||
			show_regs_safe(&stack_info, regs);
 | 
								show_regs_if_on_stack(&stack_info, regs, partial);
 | 
				
			||||||
 | 
					
 | 
				
			||||||
		/*
 | 
							/*
 | 
				
			||||||
		 * Scan the stack, printing any text addresses we find.  At the
 | 
							 * Scan the stack, printing any text addresses we find.  At the
 | 
				
			||||||
| 
						 | 
					@ -164,7 +176,7 @@ void show_trace_log_lvl(struct task_struct *task, struct pt_regs *regs,
 | 
				
			||||||
 | 
					
 | 
				
			||||||
			/*
 | 
								/*
 | 
				
			||||||
			 * Don't print regs->ip again if it was already printed
 | 
								 * Don't print regs->ip again if it was already printed
 | 
				
			||||||
			 * by show_regs_safe() below.
 | 
								 * by show_regs_if_on_stack().
 | 
				
			||||||
			 */
 | 
								 */
 | 
				
			||||||
			if (regs && stack == ®s->ip)
 | 
								if (regs && stack == ®s->ip)
 | 
				
			||||||
				goto next;
 | 
									goto next;
 | 
				
			||||||
| 
						 | 
					@ -199,9 +211,9 @@ void show_trace_log_lvl(struct task_struct *task, struct pt_regs *regs,
 | 
				
			||||||
			unwind_next_frame(&state);
 | 
								unwind_next_frame(&state);
 | 
				
			||||||
 | 
					
 | 
				
			||||||
			/* if the frame has entry regs, print them */
 | 
								/* if the frame has entry regs, print them */
 | 
				
			||||||
			regs = unwind_get_entry_regs(&state);
 | 
								regs = unwind_get_entry_regs(&state, &partial);
 | 
				
			||||||
			if (regs)
 | 
								if (regs)
 | 
				
			||||||
				show_regs_safe(&stack_info, regs);
 | 
									show_regs_if_on_stack(&stack_info, regs, partial);
 | 
				
			||||||
		}
 | 
							}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
		if (stack_name)
 | 
							if (stack_name)
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
| 
						 | 
					@ -98,7 +98,7 @@ static int __save_stack_trace_reliable(struct stack_trace *trace,
 | 
				
			||||||
	for (unwind_start(&state, task, NULL, NULL); !unwind_done(&state);
 | 
						for (unwind_start(&state, task, NULL, NULL); !unwind_done(&state);
 | 
				
			||||||
	     unwind_next_frame(&state)) {
 | 
						     unwind_next_frame(&state)) {
 | 
				
			||||||
 | 
					
 | 
				
			||||||
		regs = unwind_get_entry_regs(&state);
 | 
							regs = unwind_get_entry_regs(&state, NULL);
 | 
				
			||||||
		if (regs) {
 | 
							if (regs) {
 | 
				
			||||||
			/*
 | 
								/*
 | 
				
			||||||
			 * Kernel mode registers on the stack indicate an
 | 
								 * Kernel mode registers on the stack indicate an
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
		Loading…
	
		Reference in a new issue