linux-kernel - Re: [PATCH 1/3] x86: get rid of KERNEL_STACK

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, 19 Mar 2015 08:43:21 -0700
From:	Andy Lutomirski <luto@...capital.net>
To:	Denys Vlasenko <dvlasenk@...hat.com>
Cc:	Linus Torvalds <torvalds@...ux-foundation.org>,
	Steven Rostedt <rostedt@...dmis.org>,
	Ingo Molnar <mingo@...nel.org>, Borislav Petkov <bp@...en8.de>,
	"H. Peter Anvin" <hpa@...or.com>, Oleg Nesterov <oleg@...hat.com>,
	Frederic Weisbecker <fweisbec@...il.com>,
	Alexei Starovoitov <ast@...mgrid.com>,
	Will Drewry <wad@...omium.org>,
	Kees Cook <keescook@...omium.org>, X86 ML <x86@...nel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 1/3] x86: get rid of KERNEL_STACK_OFFSET

On Thu, Mar 19, 2015 at 8:28 AM, Denys Vlasenko <dvlasenk@...hat.com> wrote:
> On 03/18/2015 09:54 PM, Andy Lutomirski wrote:
>> On Wed, Mar 18, 2015 at 12:47 PM, Denys Vlasenko <dvlasenk@...hat.com> wrote:
>>> PER_CPU_VAR(kernel_stack) was set up in a way where it points
>>> five stack slots below the top of stack.
>>>
>>> Presumably, it was done to avoid one "sub $5*8,%rsp"
>>> in syscall/sysenter code paths, where iret frame needs to be
>>> created by hand.
>>>
>>> Ironically, none of them benefits from this optimization,
>>> since all of them need to allocate additional data on stack
>>> (struct pt_regs), so they still have to perform subtraction.
>>>
>>> This patch eliminates KERNEL_STACK_OFFSET.
>>>
>>> PER_CPU_VAR(kernel_stack) now points directly to top of stack.
>>> pt_regs allocations are adjusted to allocate iret frame as well.
>>> Hopefully we can merge it later with 32-bit specific
>>> PER_CPU_VAR(cpu_current_top_of_stack) variable...
>>>
>>> Semi-mysterious expressions THREAD_INFO(%rsp,RIP) - "why RIP??"
>>> are now replaced by more logical THREAD_INFO(%rsp,SIZEOF_PTREGS) -
>>> "calculate thread_info's address using information that
>>> rsp is SIZEOF_PTREGS bytes below the stack top".
>>>
>>> Net result in generated code is that constants in several insns
>>> are changed.
>>>
>>> This change is necessary for changing struct pt_regs creation
>>> in SYSCALL64 code path from MOV to PUSH instructions.
>>>
>>
>> Would it be reasonable to break this up into two pieces: first, remove
>> KERNEL_STACK_OFFSET from THREAD_INFO and related macros (i.e. make
>> them relative to current_top_of_stack instead)
>
> PER_CPU(cpu_current_top_of_stack) exists only in 32 bits.
> Can't use it in 64-bit code.
>
> PER_CPU(cpu_current_top_of_stack) becomes equal
> to PER_CPU(kernel_stack) only after this patch.

You could have a #define that gives the top of the stack from asm, though.

>
> I plan to clean up PER_CPU(cpu_current_top_of_stack)/
> PER_CPU(kernel_stack)/PER_CPU(tss->sp0) mess
> on top of my patches.
>
> First: this seems to be the easier way.

It's probably easier, but it's harder to review since you're making
two change at once (changing the rsp loaded during syscall and
changing the way that GET_THREAD_INFO works).

--Andy

>
> Second (why do this *after* this patch set, not in it?):
> the removal of KERNEL_STACK_OFFSET has no strong reason behind it
> unless we do MOV -> PUSH conversion - in which case it is mandatory:
> we must have RSP start at top of stack in 64-bit mode.
>
> I want to post a minimal patch set which results in PUSH conversion.
> Want to avoid polluting it with tangential goal of removing
> duplicated percpu variable.
>



-- 
Andy Lutomirski
AMA Capital Management, LLC
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/