On 6/15/21 8:52 PM, Suraj Jitindar Singh wrote:

On Wed, 2021-05-26 at 16:49 -0500, madvenka@linux.microsoft.com wrote:

quoted

From: "Madhavan T. Venkataraman" <redacted>

The unwinder should check if the return PC falls in any function that
is considered unreliable from an unwinding perspective. If it does,
mark the stack trace unreliable.

[snip]

Correct me if I'm wrong, but do you not need to move the final frame
check to before the unwinder_is_unreliable() call?

That is done in a patch series that has been merged into for-next/stacktrace branch.
When I merge this patch series with that, the final frame check will be done prior.

I have mentioned this in the cover letter:

Last stack frame
================

If a SYM_CODE function occurs in the very last frame in the stack trace,
then the stack trace is not considered unreliable. This is because there
is no more unwinding to do. Examples:

        - EL0 exception stack traces end in the top level EL0 exception
          handlers.

        - All kernel thread stack traces end in ret_from_fork().

Madhavan

Userland threads which have ret_from_fork as the last entry on the
stack will always be marked unreliable as they will always have a
SYM_CODE entry on their stack (the ret_from_fork).

Also given that this means the last frame has been reached and as such
there's no more unwinding to do, I don't think we care if the last pc
is a code address.

- Suraj

quoted

  *

@@ -133,7 +236,20 @@ int notrace unwind_frame(struct task_struct

*tsk, struct stackframe *frame)
 	 *	- Foreign code (e.g. EFI runtime services)
 	 *	- Procedure Linkage Table (PLT) entries and veneer
functions
 	 */
-	if (!__kernel_text_address(frame->pc))
+	if (!__kernel_text_address(frame->pc)) {
+		frame->reliable = false;
+		return 0;
+	}
+
+	/*
+	 * If the final frame has been reached, there is no more
unwinding
+	 * to do. There is no need to check if the return PC is
considered
+	 * unreliable by the unwinder.
+	 */
+	if (!frame->fp)
+		return 0;

if (frame->fp == (unsigned long)task_pt_regs(tsk)->stackframe)
	return -ENOENT;

quoted

+
+	if (unwinder_is_unreliable(frame->pc))
 		frame->reliable = false;
 
 	return 0;

diff --git a/arch/arm64/kernel/vmlinux.lds.S

b/arch/arm64/kernel/vmlinux.lds.S
index 7eea7888bb02..32e8d57397a1 100644

--- a/arch/arm64/kernel/vmlinux.lds.S
+++ b/arch/arm64/kernel/vmlinux.lds.S

@@ -103,6 +103,12 @@ jiffies = jiffies_64;
 #define TRAMP_TEXT
 #endif
 
+#define SYM_CODE_FUNCTIONS                                     \
+       . = ALIGN(16);                                           \
+       __sym_code_functions_start = .;                         \
+       KEEP(*(sym_code_functions))                             \
+       __sym_code_functions_end = .;
+
 /*
  * The size of the PE/COFF section that covers the kernel image,

which
  * runs from _stext to _edata, must be a round multiple of the
PE/COFF

@@ -218,6 +224,7 @@ SECTIONS
 		CON_INITCALL
 		INIT_RAM_FS
 		*(.init.altinstructions .init.bss)	/* from the

EFI stub */
+               SYM_CODE_FUNCTIONS
 	}
 	.exit.data : {
 		EXIT_DATA

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel