linux-kernel - Re: [patch V4 part 3 13/29] x86/traps: Prepare for using DEFINE

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 14 May 2020 14:16:29 +0200
From:   Thomas Gleixner <tglx@...utronix.de>
To:     Andy Lutomirski <luto@...nel.org>
Cc:     LKML <linux-kernel@...r.kernel.org>, X86 ML <x86@...nel.org>,
        "Paul E. McKenney" <paulmck@...nel.org>,
        Andy Lutomirski <luto@...nel.org>,
        Alexandre Chartre <alexandre.chartre@...cle.com>,
        Frederic Weisbecker <frederic@...nel.org>,
        Paolo Bonzini <pbonzini@...hat.com>,
        Sean Christopherson <sean.j.christopherson@...el.com>,
        Masami Hiramatsu <mhiramat@...nel.org>,
        Petr Mladek <pmladek@...e.com>,
        Steven Rostedt <rostedt@...dmis.org>,
        Joel Fernandes <joel@...lfernandes.org>,
        Boris Ostrovsky <boris.ostrovsky@...cle.com>,
        Juergen Gross <jgross@...e.com>,
        Brian Gerst <brgerst@...il.com>,
        Mathieu Desnoyers <mathieu.desnoyers@...icios.com>,
        Josh Poimboeuf <jpoimboe@...hat.com>,
        Will Deacon <will@...nel.org>
Subject: Re: [patch V4 part 3 13/29] x86/traps: Prepare for using DEFINE_IDTENTRY

Andy,

Andy Lutomirski <luto@...nel.org> writes:
> On Tue, May 5, 2020 at 7:15 AM Thomas Gleixner <tglx@...utronix.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