On 10/27/21 12:53 PM, Mark Rutland wrote:

On Thu, Oct 14, 2021 at 09:58:44PM -0500, madvenka@linux.microsoft.com wrote:

quoted

From: "Madhavan T. Venkataraman" <redacted>

Rename unwinder functions for consistency and better naming.

	- Rename start_backtrace() to unwind_start().
	- Rename unwind_frame() to unwind_next().
	- Rename walk_stackframe() to unwind().

This looks good to me.

Thanks.

Could we split this from the krpbes/tracing changes? I think this stands
on it's own, and (as below) the kprobes/tracing changes need some more
explanation, and would make sense as a separate patch.

OK. I will split the patches.

quoted

Prevent the following unwinder functions from being traced:

	- unwind_start()
	- unwind_next()

	unwind() is already prevented from being traced.

This could do with an explanation in the commis message as to why we
need to do this. If this is fixing a latent issue, it should be in a
preparatory patch that we can backport.

I dug into this a bit, and from taking a look, we prohibited ftrace in commit:

  0c32706dac1b0a72 ("arm64: stacktrace: avoid tracing arch_stack_walk()")

... which is just one special case of graph return stack unbalancing,
and should be addressed by using HAVE_FUNCTION_GRAPH_RET_ADDR_PTR, so
with the patch making us use HAVE_FUNCTION_GRAPH_RET_ADDR_PTR, that's
no longer necessary.

So we no longer seem to have a specific reason to prohibit ftrace
here.

OK, I will think about this and add a comment.

quoted

Prevent the following unwinder functions from being kprobed:

	- unwind_start()

	unwind_next() and unwind() are already prevented from being kprobed.

Likewise, I think this needs some explanation. From diggin, we
prohibited kprobes in commit:

  ee07b93e7721ccd5 ("arm64: unwind: Prohibit probing on return_address()")

... and the commit message says we need to do this because this is
(transitively) called by trace_hardirqs_off(), which is kprobes
blacklisted, but doesn't explain the actual problem this results in.

OK. I will think about this and add a comment.

AFAICT x86 directly uses __builtin_return_address() here, but that won't
recover rewritten addresses, which seems like a bug (or at least a
limitation) on x86, assuming I've read that correctly.

OK.

Thanks,

Madhavan

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