Sorry for the late reply.

On 1/28/21 8:22 AM, Mark Brown wrote:

On Wed, Jan 27, 2021 at 01:54:21PM -0600, Madhavan T. Venkataraman wrote:

Copying in Julien and Josh for the objtool stuff.  I'm replying here but
I'm not 100% up to speed yet so take what I say with a grain of salt.

quoted

FP and no-FP functions
=====================

This terminology is a bit confusing to me, I keep expanding FP to
floating point here.  Perhaps stack maintanance?

I will just say frame pointer.

quoted

I have a suggestion for objtool and the unwinder for ARM64.

IIUC, objtool is responsible for walking all the code paths (except unreachable
and ignored ones) and making sure that every function has proper frame pointer
code (prolog, epilog, etc). If a function is found to not have it, the kernel
build is failed. Is this understanding correct?

Roughly, AIUI.

quoted

If so, can we take a different approach for ARM64?

This feels like something that should just be a stack protector feature
rather than something that an individual architecture should be striking
out on its own with.

Yes, it is a stack feature. It may mean less work that objtool has to do
for reliable stack trace. I am not trying to eliminate objtool.

quoted

Instead of failing the kernel build, can we just mark the functions as:

quoted

	FP	Functions that have proper FP code
	no-FP	Functions that don't

quoted

May be, we can add an "FP" flag in the symbol table entry for this.

quoted

Then, the unwinder can check the functions it encounters in the stack trace and
inform the caller if it found any no-FP functions. The caller of the unwinder can
decide what he wants to do with that information.

	- the caller can ignore it

	- the caller can print the stack trace with a warning that no-FP functions
	  were found

	- if the caller is livepatch, the caller can retry until the no-FP functions
	  disappear from the stack trace. This way, we can have live patching even
	  when some of the functions in the kernel are no-FP.

Does this make any sense? Is this acceptable? What are the pitfalls?

One downside of this would be that we'd need some way of figuring out if
we've got enough of the kernel covered to be actually useful for
livepatch, and it does mean that we'll have some percentage of code
where the debugging uses for unwinding will be impacted.

Agreed. I am not actually saying that no-frame pointer functions should not be
fixed. We should really fix them. But some may not be fixable because of their
nature. And, some may be deferred because of their complexity. That should not
be a show stopper for livepatch.

It does seem like it's broadly the distinction between the existing
standard and reliable stacktrace interfaces but pushed into the
callback, however reliable stacktrace is going to also perform
additional checks like looking for conditions that might leave things in
an inconsistent state (eg, finding probes or interrupts being taken) so
the distinction isn't just in the binary but also other runtime
conditions.  Reliable stacktrace can also make assumptions that the task
is not currently running unless it is the task doing the trace.  

Agreed.

There was actually until very recently a flag on the callback that is
provided to arch_stack_walk() which indicated if things were supposed to
be reliable but there were no users and nothing ever changed the value,
there was something in a comment or changelog somewhere which said that
this had been for the benefit of printk() but the user had never been
merged.  That sounds a lot like what you're suggesting.

I will take a look.

quoted

If we can do this, the unwinder could detect cases such as:

quoted

- If gcc thinks that a function is a leaf function but the function contains
  inline assembly code that calls another function.

quoted

- If a call to a function bounces through some intermediate code such as a
  trampoline.

quoted

- etc.

quoted

For specific no-FP functions, the unwinder might be able to deduce the original
caller. In these cases, the stack trace would still be reliable. For all the others,
the stack trace would be considered unreliable.

I'm not entirely sure I see the distinction from the current situation
here?

Currently, the unwinder does not know that a function that it encounters in the stack
trace is no-frame pointer. No-frame pointer functions hide their callers in the stack
trace. Objtool addresses that problem through static analysis.

I am proposing that we do this detection dynamically so we can do livepatching even
if the kernel has no-frame pointer functions.

quoted

Compiler instead of objtool
===========================

If the above suggestion is acceptable, I have another suggestion.

It is a lot of work for every new architecture to add frame pointer verification
support in objtool. Can we get some help from the compiler?

The compiler knows which C functions it generates the FP prolog and epilog for. It can
mark those functions as FP. As for assembly functions, kernel developers could manually
annotate functions that have proper FP code. The compiler/assembler would mark them
as FP. Only a small subset of assembly functions would even have FP prolog and epilog.

quoted

Is this acceptable? What are the pitfalls?

If we're trusting the compiler we can probably just do that without any
explicit support from the compiler - it should be doing the standard
stuff unless we explicitly ask it to and if it isn't then it might be a
result of a mismatch in assumptions rather than a deliberate decision to
do something non-standard.  My understanding with objtool is that a big
part of the idea is to provide a static check that the binary we end up
with matches the assumptions that we are making so the fact that it's a
separate implementation is important.

Agreed. Static checks are necessary. I am only trying to see if the number
of static checks we have to do in objtool can be minimized. Also, should
every static check failure fail the kernel build?

Madhavan

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