Re: boot flooded with unwind: Index not found

Ard Biesheuvel
2022-03-02T11:20:40Z

On Wed, 2 Mar 2022 at 12:12, Russell King (Oracle)
<linux@armlinux.org.uk> wrote:
>
> On Wed, Mar 02, 2022 at 11:09:49AM +0100, Corentin Labbe wrote:
> > The crash disappeared (but the suspicious RCU usage is still here).
>
> As the trace on those is:
>
> [ 0.239629] unwind_backtrace from show_stack+0x10/0x14
> [ 0.239654] show_stack from init_stack+0x1c54/0x2000
>
> unwind_backtrace() and show_stack() are both C code, the compiler will
> emit the unwind information for it. show_stack() isn't called from
> assembly code, only from C code, so the next function's unwind
> information should also be generated by the compiler.
>
> However, init_stack is not a function - it's an array of unsigned long.
> There is no way this should appear in the trace, and this suggests that
> the unwind of show_stack() has gone wrong.
>
> I don't see anything obvious in Ard's changes that would cause that
> though.
>
> Did it used to work fine with previous versions of linux-next - those
> versions where we had Ard's "arm-vmap-stacks-v6" tag merged in
> (commit 2fa394824493) and did this only appear when I merged
> "arm-ftrace-for-rmk" (commit 74aaaa1e9bba) ? Did merging
> "arm-ftrace-for-rmk" cause any change in your .config?
>

I can reproduce the RCU warnings, and I have tracked this down to the
change I made to return_address() for the graph tracer, which I
thought was justified after removing the call to
kernel_text_address():

--- a/arch/arm/include/asm/ftrace.h
+++ b/arch/arm/include/asm/ftrace.h
@@ -35,26 +35,8 @@ static inline unsigned long
ftrace_call_adjust(unsigned long addr)

#ifndef __ASSEMBLY__

-#if defined(CONFIG_FRAME_POINTER) && !defined(CONFIG_ARM_UNWIND)
-/*
- * return_address uses walk_stackframe to do it's work. If both
- * CONFIG_FRAME_POINTER=y and CONFIG_ARM_UNWIND=y walk_stackframe uses unwind
- * information. For this to work in the function tracer many functions would
- * have to be marked with __notrace. So for now just depend on
- * !CONFIG_ARM_UNWIND.
- */
-
void *return_address(unsigned int);

-#else
-
-static inline void *return_address(unsigned int level)
-{
- return NULL;
-}
-
-#endif
-
#define ftrace_return_address(n) return_address(n)

#define ARCH_HAS_SYSCALL_MATCH_SYM_NAME

However, the function graph tracer works happily with this bit
reverted, and so that is probably the best course of action here.

I have already sent the patch that reintroduces the
kernel_text_address() check - would you prefer a v2 of that one with
this change incorporated? Or a second patch that just reverts the
above? (Given that the bogus dereference was invoked from
return_address() as well, I suspect that this change would make the
get_kernel_nofault() change I proposed in this thread redundant)