linux-kernel - Re: [PATCH 3/4] x86: save user rsp in pt

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: <CALCETrUFbbjeUrZEuUvGQLefrKBTH=RTdVJ0vXgAGBEEuzEMKg@mail.gmail.com>
Date:	Tue, 10 Mar 2015 06:10:29 -0700
From:	Andy Lutomirski <luto@...capital.net>
To:	Ingo Molnar <mingo@...nel.org>
Cc:	Denys Vlasenko <dvlasenk@...hat.com>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Steven Rostedt <rostedt@...dmis.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 3/4] x86: save user rsp in pt_regs->sp on SYSCALL64 fastpath

On Tue, Mar 10, 2015 at 5:51 AM, Ingo Molnar <mingo@...nel.org> wrote:
>
> * Denys Vlasenko <dvlasenk@...hat.com> wrote:
>
>> PER_CPU(old_rsp) usage is simplified - now it is used only
>> as temp storage, and userspace stack pointer is immediately stored
>> in pt_regs->sp on syscall entry, instead of being used later,
>> on syscall exit.
>>
>> Instead of PER_CPU(old_rsp) and task->thread.usersp, C code
>> uses pt_regs->sp now.
>>
>> FIXUP/RESTORE_TOP_OF_STACK are simplified.
>
> Just trying to judge the performance impact:
>
>> --- a/arch/x86/kernel/entry_64.S
>> +++ b/arch/x86/kernel/entry_64.S
>> @@ -128,8 +128,6 @@ ENDPROC(native_usergs_sysret64)
>>   * manipulation.
>>   */
>>       .macro FIXUP_TOP_OF_STACK tmp offset=0
>> -     movq PER_CPU_VAR(old_rsp),\tmp
>> -     movq \tmp,RSP+\offset(%rsp)
>>       movq $__USER_DS,SS+\offset(%rsp)
>>       movq $__USER_CS,CS+\offset(%rsp)
>>       movq RIP+\offset(%rsp),\tmp  /* get rip */
>> @@ -139,8 +137,7 @@ ENDPROC(native_usergs_sysret64)
>>       .endm
>>
>>       .macro RESTORE_TOP_OF_STACK tmp offset=0
>> -     movq RSP+\offset(%rsp),\tmp
>> -     movq \tmp,PER_CPU_VAR(old_rsp)
>> +     /* nothing to do */
>>       .endm
>>
>>  /*
>> @@ -253,11 +247,13 @@ GLOBAL(system_call_after_swapgs)
>>        */
>>       ENABLE_INTERRUPTS(CLBR_NONE)
>>       ALLOC_PT_GPREGS_ON_STACK 8              /* +8: space for orig_ax */
>> +     movq    %rcx,RIP(%rsp)
>> +     movq    PER_CPU_VAR(old_rsp),%rcx
>> +     movq    %r11,EFLAGS(%rsp)
>> +     movq    %rcx,RSP(%rsp)
>> +     movq_cfi rax,ORIG_RAX
>>       SAVE_C_REGS_EXCEPT_RAX_RCX_R11
>>       movq    $-ENOSYS,RAX(%rsp)
>> -     movq_cfi rax,ORIG_RAX
>> -     movq    %r11,EFLAGS(%rsp)
>> -     movq    %rcx,RIP(%rsp)
>>       CFI_REL_OFFSET rip,RIP
>>       testl $_TIF_WORK_SYSCALL_ENTRY,TI_flags+THREAD_INFO(%rsp,RIP)
>>       jnz tracesys
>
> So there are now +2 instructions (5 instead of 3) in the system_call
> path, but there are -2 instructions in the SYSRETQ path, so combined
> it's a wash performance-wise, right?

I think that the SYSRETQ path is the same number of instructions --
RESTORE_TOP_OF_STACK is only used in somewhat unusual circumstances.

On SYSRETQ, I think we touch one fewer cache line, although that cache
line will be in L1 unless the syscall pushed it out.  The added
instructions on entry are probably nearly free, though, as one of them
is a load from a forwarded address and the other is a store to a hot
cacheline.

--And
--
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/