Re: [PATCH 20/22] x86,word-at-a-time: Remove .fixup usage

On Fri, 12 Nov 2021, Josh Poimboeuf wrote:

On Fri, Nov 12, 2021 at 10:33:36AM +0100, Peter Zijlstra wrote:

quoted

On Thu, Nov 11, 2021 at 05:50:03PM -0800, Josh Poimboeuf wrote:

quoted

Hm, I think there is actually a livepatch problem here.

I suspected as much, because I couldn't find any code dealing with it
when I looked in a hurry.. :/

quoted

Some ideas to fix:

quoted

c) Update the reliable stacktrace code to mark the stack unreliable if
   it has a function with ".cold" in the name?

Why not simply match func.cold as func in the transition thing? Then
func won't get patched as long as it (or it's .cold part) is in use.
This seems like the natural thing to do.

Well yes, you're basically hinting at my first two options a and b:

a) Add a field to 'klp_func' which allows the patch module to specify a
   function's .cold counterpart?

b) Detect such cold counterparts in klp_enable_patch()?  Presumably it
   would require searching kallsyms for "<func>.cold", which is somewhat
   problematic as there might be duplicates.

It's basically a two-step process:  1) match func to .cold if it exists;
2) check for both in klp_check_stack_func().  The above two options are
proposals for the 1st step.  The 2nd step was implied.

This reminded me... one of the things I have on my todo list for a long 
time is to add an option for a live patch creator to specify functions 
which are not contained in the live patch but their presence on stacks 
should be checked for. It could save some space in the final live patch 
when one would add functions (callers) just because the consistency 
requires it.

I took as a convenience feature with a low priority and forgot about it. 
The problem above changes it. So should we take the opportunity and 
implement both in one step? I wanted to include a list of functions in 
on a patch level (klp_patch structure) and klp_check_stack() would just 
have more to check.

Miroslav