linux-kernel - Re: [patch 00/15] x86/entry: Consolidation

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: <20200226100256.GK14946@hirez.programming.kicks-ass.net>
Date:   Wed, 26 Feb 2020 11:02:56 +0100
From:   Peter Zijlstra <peterz@...radead.org>
To:     Thomas Gleixner <tglx@...utronix.de>
Cc:     LKML <linux-kernel@...r.kernel.org>, x86@...nel.org,
        Steven Rostedt <rostedt@...dmis.org>,
        Brian Gerst <brgerst@...il.com>,
        Juergen Gross <jgross@...e.com>,
        Paolo Bonzini <pbonzini@...hat.com>,
        Arnd Bergmann <arnd@...db.de>
Subject: Re: [patch 00/15] x86/entry: Consolidation - Part V

On Wed, Feb 26, 2020 at 10:53:19AM +0100, Peter Zijlstra wrote:
> +SYM_CODE_START_LOCAL_NOALIGN(common_idtentry)
> +	/* the return address is in the %gs stack slot */
>  	SAVE_ALL switch_stacks=1 skip_gs=1 unwind_espfix=1
>  	ENCODE_FRAME_POINTER
>  
>  	/* fixup %gs */
>  	GS_TO_REG %ecx
> -	movl	PT_GS(%esp), %edi		# get the function address
> -	REG_TO_PTGS %ecx
> +	pushl	PT_GS(%esp)			# push return address
> +	REG_TO_OTGS %ecx

Aside from the obvious typo, it is also completely broken because
REG_TO_PGTS relies on the stack layout, which we just wrecked.

	movl	PT_GS(%esp), %edi		# get the return address
	REG_TO_PTGS %ecx

>  	SET_KERNEL_GS %ecx
>  
>  	/* fixup orig %eax */
> @@ -1348,9 +1348,8 @@ SYM_CODE_START_LOCAL_NOALIGN(common_exception)
>  	movl	$-1, PT_ORIG_EAX(%esp)		# no syscall to restart
>  
>  	movl	%esp, %eax			# pt_regs pointer

	pushl	%edi
> +	ret
> +SYM_CODE_END(common_idtentry)

Should work, although that push+ret combo is a bit awkward.