linux-kernel - Re: [patch V4 part 5 02/31] x86/entry: Provide helpers for execute on irqstack

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <87pnbho4o5.fsf@nanos.tec.linutronix.de>
Date:   Wed, 06 May 2020 10:20:42 +0200
From:   Thomas Gleixner <tglx@...utronix.de>
To:     LKML <linux-kernel@...r.kernel.org>
Cc:     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 5 02/31] x86/entry: Provide helpers for execute on irqstack

Thomas Gleixner <tglx@...utronix.de> writes:
> That also allows to move the xen hypercall extra magic code and the softirq
> stack switching into C.
>
> The mechanism is straight forward:
>
>   1) Store the current stack pointer on top of the interrupt stack. That's
>      required for the unwinder. 
>
>   2) Switch the stack pointer
>
>   3) Call the function
>
>   4) Restore the stackpointer
>
> The full code sequence to make the unwinder happy is:
>
>     	pushq	%rbp
> 	movq	%rsp, %rbp
> 	movq    $(top_of_hardirq_stack - 8), %reg
> 	movq	%rsp, (%reg)
>   	movq	%reg , %rsp
> 	call    function
> 	popq	%rsp
> 	leaveq
> 	
> While the following sequence would spare the 'popq %rsp':
>
>     	pushq	%rbp
> 	movq    $(top_of_hardirq_stack - 8), %rbp
> 	movq	%rsp, (%rrbp)
>   	xchgq	%rbp, %rsp
> 	call    function
>  	movq	%rbp, %rsp
> 	leaveq

So I stared some more into that.

The push rbp is wrong for the frame unwinder case. That one is happy
(except for objtool) with the most minimalistic variant:

	movq	%%rsp, (%[tos])
        movq	%[tos], %%rsp
        call	function
        popq	%%rsp

which is not surprising because for the frame unwinder this is similar
to the 'gcc aligns stack in the middle of the function' handling. BP
still has to point to the previous frame. Adjustment of BP must only
happen on function entry.

The stack border convention of having the pointer to the previous stack
in the top word is sufficient for this.

objtool complains though:

 warning: objtool: do_softirq_own_stack()+0x67: return with modified stack frame

That obviously makes also the ORC unwinder unhappty as objtool fails to
provide the right hint. But also for ORC this construct should be
completely sufficient.

I'm exploring another idea right now, but wanted to share the info.

Thanks,

        tglx