Messages in this thread | | | Date | Sat, 29 Apr 2023 11:38:26 +0800 | From | "Hou Wenlong" <> | Subject | Re: [PATCH RFC 22/43] x86/ftrace: Adapt ftrace nop patching for PIE support |
| |
On Fri, Apr 28, 2023 at 09:44:54PM +0800, Steven Rostedt wrote: > On Fri, 28 Apr 2023 17:51:02 +0800 > "Hou Wenlong" <houwenlong.hwl@antgroup.com> wrote: > > > From: Thomas Garnier <thgarnie@chromium.org> > > > > From: Thomas Garnier <thgarnie@chromium.org> > > > > When using PIE with function tracing, the compiler generates a > > call through the GOT (call *__fentry__@GOTPCREL). This instruction > > takes 6-bytes instead of 5-bytes with a relative call. And -mnop-mcount > > option is not implemented for -fPIE now. > > > > If PIE is enabled, replace the 6th byte of the GOT call by a 1-byte nop > > so ftrace can handle the previous 5-bytes as before. > > Wait! This won't work! > > You can't just append another nop to fill in the blanks here. We must > either have a single 6 byte nop, or we need to refactor the entire logic to > something that other archs have. > > The two nops means that the CPU can take it as two separate commands. > There's nothing stopping the computer from preempting a task between the > two. If that happens, and you modify the 1byte nop and 5byte nop with a > single 6 byte command, when the task get's rescheduled, it will execute the > last 5 bytes of that 6 byte command and take a general protection fault, and > likely crash the machine. > > NACK on this. It needs a better solution. > > -- Steve > > Hi Steve,
Sorry for not providing the original patch link: https://lore.kernel.org/all/20190131192533.34130-22-thgarnie@chromium.org/
I drop the Reviewed-by tag due to the change described in commit message.
This nop patching is only used for the first time (addr = MCOUNT) before SMP or executing code in module. And ftrace_make_call() is not modified, then we would use 5 byte direct call to replace the first 5 byte nop when tracepoint is enabled like before, it's still one instruction. So, the logic is same like before, patch the first 5 byte when tracepoint is enabled or disabled during running.
> > > > [Hou Wenlong: Adapt code change and fix wrong offset calculation in > > make_nop_x86()] > >
| |