Messages in this thread | | | From | Thomas Gleixner <> | Subject | Re: [patch V4 part 3 13/29] x86/traps: Prepare for using DEFINE_IDTENTRY | Date | Thu, 14 May 2020 14:16:29 +0200 |
| |
Andy,
Andy Lutomirski <luto@kernel.org> writes: > On Tue, May 5, 2020 at 7:15 AM Thomas Gleixner <tglx@linutronix.de> wrote: >> >> +static __always_inline void __user *error_get_trap_addr(struct pt_regs *regs) >> +{ >> + return (void __user *)uprobe_get_trap_addr(regs); >> +} > > My mind boggles. WTF is this?
That's an inline version of the same nonsense in that macro maze.
> Perhaps this should have a comment like: > > /* > * Returns the address from which a user trap originated. This would be the same as regs->ip, > * except for frhgnieawfn nvlrkvklsrvs and mfkealwf, and this lets the thingummy pass a > * modified value to the signal frame, but only for #DE and #UD, because #*!&@&#@. > */ > > except with the blanks filled in.
Hmm. Filling the blanks gives me:
/* **0Returns1the2address3from4which5a6user7trap8originated.90This1would...
That's not really more readable than yours :)
> After reading a bit of uprobe code, I assume this is a fixup for when > we're running code that got moved out of line because it got replaced > by a breakpoint. I'm still mystified by why it only seems to apply to > #DE and #UD.
man sigaction(2):
* SIGILL, SIGFPE, SIGSEGV, SIGBUS, and SIGTRAP fill in si_addr with the address of the fault. On some architectures, these signals also fill in the si_trapno field.
Posix spec says:
In addition, the following signal-specific information shall be available:
Signal Member Value
SIGILL void * si_addr Address of faulting instruction. SIGFPE SIGSEGV void * si_addr Address of faulting memory reference. SIGBUS
For some implementations, the value of si_addr may be inaccurate.
In historic kernels si_addr was simply set to regs->ip and the uprobe muck changed that in commit b02ef20a9fba08 ("uprobes/x86: Fix the wrong ->si_addr when xol triggers a trap") If the probed insn triggers a trap, ->si_addr = regs->ip is technically correct, but this is not what the signal handler wants; we need to pass the address of the probed insn, not the address of xol slot.
Now that I filled my own blanks, I think that I can come up with a halfways useful comment.
Thanks,
tglx
| |