Re: [PATCH v2] x86/show_trace_log_lvl: ensure stack pointer is aligned, again

[Oleg Nesterov <oleg@redhat.com>]

Josh,

This version does exactly what you requested in reply to v1.

	https://lore.kernel.org/lkml/20230428043158.r5omehiaqawcac2y@treble/

What else should we do? Make V3 following David's suggestion

	https://lore.kernel.org/lkml/aa8537d81dc747a097e9e30491b5081b@AcuMS.aculab.com/

? Or what?

On 04/29, Oleg Nesterov wrote:
>
> From: Vernon Lovejoy <vlovejoy@redhat.com>
> 
> The commit e335bb51cc15 ("x86/unwind: Ensure stack pointer is aligned")
> tried to align the stack pointer in show_trace_log_lvl(), otherwise the
> "stack < stack_info.end" check can't guarantee that the last read does
> not go past the end of the stack.
> 
> However, we have the same problem with the initial value of the stack
> pointer, it can also be unaligned. So without this patch this trivial
> kernel module
> 
> 	#include <linux/module.h>
> 
> 	static int init(void)
> 	{
> 		asm volatile("sub    $0x4,%rsp");
> 		dump_stack();
> 		asm volatile("add    $0x4,%rsp");
> 
> 		return -EAGAIN;
> 	}
> 
> 	module_init(init);
> 	MODULE_LICENSE("GPL");
> 
> crashes the kernel.
> 
> Fixes: e335bb51cc15 ("x86/unwind: Ensure stack pointer is aligned")
> Signed-off-by: Vernon Lovejoy <vlovejoy@redhat.com>
> Signed-off-by: Oleg Nesterov <oleg@redhat.com>
> ---
>  arch/x86/kernel/dumpstack.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/kernel/dumpstack.c b/arch/x86/kernel/dumpstack.c
> index 0bf6779187dd..f82d896eb99d 100644
> --- a/arch/x86/kernel/dumpstack.c
> +++ b/arch/x86/kernel/dumpstack.c
> @@ -195,7 +195,6 @@ static void show_trace_log_lvl(struct task_struct *task, struct pt_regs *regs,
>  	printk("%sCall Trace:\n", log_lvl);
>  
>  	unwind_start(&state, task, regs, stack);
> -	stack = stack ? : get_stack_pointer(task, regs);
>  	regs = unwind_get_entry_regs(&state, &partial);
>  
>  	/*
> @@ -214,7 +213,10 @@ static void show_trace_log_lvl(struct task_struct *task, struct pt_regs *regs,
>  	 * - hardirq stack
>  	 * - entry stack
>  	 */
> -	for ( ; stack; stack = PTR_ALIGN(stack_info.next_sp, sizeof(long))) {
> +	for (stack = PTR_ALIGN(stack ?: get_stack_pointer(task, regs), sizeof(long));
> +	     stack;
> +	     stack = PTR_ALIGN(stack_info.next_sp, sizeof(long)))
> +	{
>  		const char *stack_name;
>  
>  		if (get_stack_info(stack, task, &stack_info, &visit_mask)) {
> -- 
> 2.25.1.362.g51ebf55
>