diff mbox

[2/2] x86-64/swapcontext: Restore the pointer into %rdx after syscall

Message ID 20180426121744.GB30766@intel.com
State New, archived
Headers

Commit Message

Lu, Hongjiu April 26, 2018, 12:17 p.m. UTC 
  To prepare for shadow stack support, restore the pointer into %rdx after
syscall and use %rdx, instead of %rsi, to restore context.  There is no
functional change.

Any comments?

H.J.
----
	* sysdeps/unix/sysv/linux/x86_64/swapcontext.S (__swapcontext):
	Restore the pointer into %rdx, after syscall and use %rdx,
	instead of %rsi, to restore context.
---
 sysdeps/unix/sysv/linux/x86_64/swapcontext.S | 41 ++++++++++++++--------------
 1 file changed, 21 insertions(+), 20 deletions(-)

Comments

Florian Weimer April 29, 2018, 9:10 p.m. UTC | #1 
On 04/26/2018 02:17 PM, H.J. Lu wrote:
> +	/* Restore destroyed register into RDX which is preserved by
> +	   the syscall.  */
> +	movq	%r12, %rdx

Again, this comment does not make sense without the other upcoming changes.

Thanks,
Florian
H.J. Lu April 29, 2018, 10:08 p.m. UTC | #2 
On Sun, Apr 29, 2018 at 2:10 PM, Florian Weimer <fweimer@redhat.com> wrote:
> On 04/26/2018 02:17 PM, H.J. Lu wrote:
>>
>> +       /* Restore destroyed register into RDX which is preserved by
>> +          the syscall.  */
>> +       movq    %r12, %rdx
>
>
> Again, this comment does not make sense without the other upcoming changes.
>

Here is the patch to use it:

https://github.com/hjl-tools/glibc/commit/182ce0987aada98b2d606276d4aa4fbd2527031b
Carlos O'Donell May 2, 2018, 4:46 a.m. UTC | #3 
On 04/29/2018 06:08 PM, H.J. Lu wrote:
> On Sun, Apr 29, 2018 at 2:10 PM, Florian Weimer <fweimer@redhat.com> wrote:
>> On 04/26/2018 02:17 PM, H.J. Lu wrote:
>>>
>>> +       /* Restore destroyed register into RDX which is preserved by
>>> +          the syscall.  */
>>> +       movq    %r12, %rdx
>>
>>
>> Again, this comment does not make sense without the other upcoming changes.
>>
> 
> Here is the patch to use it:
> 
> https://github.com/hjl-tools/glibc/commit/182ce0987aada98b2d606276d4aa4fbd2527031b
 
If we are doing a "flag-day" enable for ucontext, is there
any reason not to add additional space today that can be
used in the future?
Carlos O'Donell May 2, 2018, 5:04 a.m. UTC | #4 
On 04/29/2018 05:10 PM, Florian Weimer wrote:
> On 04/26/2018 02:17 PM, H.J. Lu wrote:
>> +    /* Restore destroyed register into RDX which is preserved by
>> +       the syscall.  */
>> +    movq    %r12, %rdx
> 
> Again, this comment does not make sense without the other upcoming changes.

OK with my suggestion for comment in 1/2 of this series.

Reviewed-by: Carlos O'Donell <carlos@redhat.com>
H.J. Lu May 2, 2018, 1:35 p.m. UTC | #5 
On Tue, May 1, 2018 at 9:46 PM, Carlos O'Donell <carlos@redhat.com> wrote:
> On 04/29/2018 06:08 PM, H.J. Lu wrote:
>> On Sun, Apr 29, 2018 at 2:10 PM, Florian Weimer <fweimer@redhat.com> wrote:
>>> On 04/26/2018 02:17 PM, H.J. Lu wrote:
>>>>
>>>> +       /* Restore destroyed register into RDX which is preserved by
>>>> +          the syscall.  */
>>>> +       movq    %r12, %rdx
>>>
>>>
>>> Again, this comment does not make sense without the other upcoming changes.
>>>
>>
>> Here is the patch to use it:
>>
>> https://github.com/hjl-tools/glibc/commit/182ce0987aada98b2d606276d4aa4fbd2527031b
>
> If we are doing a "flag-day" enable for ucontext, is there
> any reason not to add additional space today that can be
> used in the future?

That is a good idea.  I added 4 words for CET.  We can add another 8
or 16 words.
diff mbox

Patch

diff --git a/sysdeps/unix/sysv/linux/x86_64/swapcontext.S b/sysdeps/unix/sysv/linux/x86_64/swapcontext.S
index e577c209b9..e2bb50308a 100644
--- a/sysdeps/unix/sysv/linux/x86_64/swapcontext.S
+++ b/sysdeps/unix/sysv/linux/x86_64/swapcontext.S
@@ -79,38 +79,39 @@  ENTRY(__swapcontext)
 	cmpq	$-4095, %rax		/* Check %rax for error.  */
 	jae	SYSCALL_ERROR_LABEL	/* Jump to error handler if error.  */
 
-	/* Restore destroyed registers.  */
-	movq	%r12, %rsi
+	/* Restore destroyed register into RDX which is preserved by
+	   the syscall.  */
+	movq	%r12, %rdx
 
 	/* Restore the floating-point context.  Not the registers, only the
 	   rest.  */
-	movq	oFPREGS(%rsi), %rcx
+	movq	oFPREGS(%rdx), %rcx
 	fldenv	(%rcx)
-	ldmxcsr oMXCSR(%rsi)
+	ldmxcsr oMXCSR(%rdx)
 
 	/* Load the new stack pointer and the preserved registers.  */
-	movq	oRSP(%rsi), %rsp
-	movq	oRBX(%rsi), %rbx
-	movq	oRBP(%rsi), %rbp
-	movq	oR12(%rsi), %r12
-	movq	oR13(%rsi), %r13
-	movq	oR14(%rsi), %r14
-	movq	oR15(%rsi), %r15
+	movq	oRSP(%rdx), %rsp
+	movq	oRBX(%rdx), %rbx
+	movq	oRBP(%rdx), %rbp
+	movq	oR12(%rdx), %r12
+	movq	oR13(%rdx), %r13
+	movq	oR14(%rdx), %r14
+	movq	oR15(%rdx), %r15
 
 	/* The following ret should return to the address set with
 	getcontext.  Therefore push the address on the stack.  */
-	movq	oRIP(%rsi), %rcx
+	movq	oRIP(%rdx), %rcx
 	pushq	%rcx
 
 	/* Setup registers used for passing args.  */
-	movq	oRDI(%rsi), %rdi
-	movq	oRDX(%rsi), %rdx
-	movq	oRCX(%rsi), %rcx
-	movq	oR8(%rsi), %r8
-	movq	oR9(%rsi), %r9
-
-	/* Setup finally  %rsi.  */
-	movq	oRSI(%rsi), %rsi
+	movq	oRDI(%rdx), %rdi
+	movq	oRSI(%rdx), %rsi
+	movq	oRCX(%rdx), %rcx
+	movq	oR8(%rdx), %r8
+	movq	oR9(%rdx), %r9
+
+	/* Setup finally %rdx.  */
+	movq	oRDX(%rdx), %rdx
 
 	/* Clear rax to indicate success.  */
 	xorl	%eax, %eax