Patchwork [v2,07/21] nptl: powerpc: Fix Race conditions in pthread cancellation (BZ#12683)

login
register
mail settings
Submitter Adhemerval Zanella Netto
Date Feb. 26, 2018, 9:03 p.m.
Message ID <1519679016-12241-8-git-send-email-adhemerval.zanella@linaro.org>
Download mbox | patch
Permalink /patch/26080/
State Changes Requested
Delegated to: Tulio Magno Quites Machado Filho
Headers show

Comments

Adhemerval Zanella Netto - Feb. 26, 2018, 9:03 p.m.
This patches adds the powerpc64 modification required for the BZ#12683.
It basically adds the required __pthread_get_pc function and a arch
specific syscall_cancel implementation.

The powerpc requires an arch-specific syscall_cancel because
INTERNAL_SYSCALL_NCS adds a mfcr just after the sc instruction to get
the CR0.SO bit information from kernel (which signals the error
return status).  So for cancelled syscalls with side effects,
__pthread_get_pc will point to mcfr and thus invalidating the checks
on sigcancel_handler.

Checked on powerpc64le-linux-gnu and powerpc-linux-gnu.

	[BZ #12683]
	* sysdeps/unix/sysv/linux/powerpc/syscall_cancel.S: New file.
	* sysdeps/unix/sysv/linux/powerpc/sysdep-cancel.h
	(__pthread_get_pc): New function.
---
 ChangeLog                                        |  5 ++
 sysdeps/unix/sysv/linux/powerpc/sigcontextinfo.h | 16 ++++++
 sysdeps/unix/sysv/linux/powerpc/syscall_cancel.S | 64 ++++++++++++++++++++++++
 3 files changed, 85 insertions(+)
 create mode 100644 sysdeps/unix/sysv/linux/powerpc/syscall_cancel.S
Tulio Magno Quites Machado Filho - May 7, 2018, 7:25 p.m.
Adhemerval Zanella <adhemerval.zanella@linaro.org> writes:

> This patches adds the powerpc64 modification required for the BZ#12683.
> It basically adds the required __pthread_get_pc function and a arch
> specific syscall_cancel implementation.
>
> The powerpc requires an arch-specific syscall_cancel because
> INTERNAL_SYSCALL_NCS adds a mfcr just after the sc instruction to get
> the CR0.SO bit information from kernel (which signals the error
> return status).  So for cancelled syscalls with side effects,
> __pthread_get_pc will point to mcfr and thus invalidating the checks
> on sigcancel_handler.
>
> Checked on powerpc64le-linux-gnu and powerpc-linux-gnu.
>
> 	[BZ #12683]
> 	* sysdeps/unix/sysv/linux/powerpc/syscall_cancel.S: New file.
> 	* sysdeps/unix/sysv/linux/powerpc/sysdep-cancel.h
> 	(__pthread_get_pc): New function.

This entry is outdated.  Both the file name and the function name need to be
updated.

> diff --git a/sysdeps/unix/sysv/linux/powerpc/syscall_cancel.S b/sysdeps/unix/sysv/linux/powerpc/syscall_cancel.S
> new file mode 100644
> index 0000000..2e56c72
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/powerpc/syscall_cancel.S
> @@ -0,0 +1,64 @@
> +/* Cancellable syscall wrapper.  Linux/powerpc version.
> +   Copyright (C) 2017 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <http://www.gnu.org/licenses/>.  */
> +
> +#include <sysdep.h>
> +
> +/* long int [r3] __syscall_cancel_arch (int *cancelhandling [r3],
> +					long int nr   [r4],
> +					long int arg1 [r5],
> +					long int arg2 [r6],
> +					long int arg3 [r7],
> +					long int arg4 [r8],
> +					long int arg5 [r9],
> +					long int arg6 [r10])  */
> +
> +ENTRY (__syscall_cancel_arch)
> +
> +	.globl __syscall_cancel_arch_start
> +	.type  __syscall_cancel_arch_start,@function
> +__syscall_cancel_arch_start:
> +
> +	/* if (*cancelhandling & CANCELED_BITMASK)
> +	     __syscall_do_cancel()  */
> +	lwz     r0,0(r3)
> +	rldicl. r0,r0,62,63
> +	beq     1f
> +	b       __syscall_do_cancel

Should this be a Branch & Link?
Adhemerval Zanella Netto - May 8, 2018, 6:07 p.m.
On 07/05/2018 16:25, Tulio Magno Quites Machado Filho wrote:
> Adhemerval Zanella <adhemerval.zanella@linaro.org> writes:
> 
>> This patches adds the powerpc64 modification required for the BZ#12683.
>> It basically adds the required __pthread_get_pc function and a arch
>> specific syscall_cancel implementation.
>>
>> The powerpc requires an arch-specific syscall_cancel because
>> INTERNAL_SYSCALL_NCS adds a mfcr just after the sc instruction to get
>> the CR0.SO bit information from kernel (which signals the error
>> return status).  So for cancelled syscalls with side effects,
>> __pthread_get_pc will point to mcfr and thus invalidating the checks
>> on sigcancel_handler.
>>
>> Checked on powerpc64le-linux-gnu and powerpc-linux-gnu.
>>
>> 	[BZ #12683]
>> 	* sysdeps/unix/sysv/linux/powerpc/syscall_cancel.S: New file.
>> 	* sysdeps/unix/sysv/linux/powerpc/sysdep-cancel.h
>> 	(__pthread_get_pc): New function.
> 
> This entry is outdated.  Both the file name and the function name need to be
> updated.

Ack.

> 
>> diff --git a/sysdeps/unix/sysv/linux/powerpc/syscall_cancel.S b/sysdeps/unix/sysv/linux/powerpc/syscall_cancel.S
>> new file mode 100644
>> index 0000000..2e56c72
>> --- /dev/null
>> +++ b/sysdeps/unix/sysv/linux/powerpc/syscall_cancel.S
>> @@ -0,0 +1,64 @@
>> +/* Cancellable syscall wrapper.  Linux/powerpc version.
>> +   Copyright (C) 2017 Free Software Foundation, Inc.
>> +   This file is part of the GNU C Library.
>> +
>> +   The GNU C Library is free software; you can redistribute it and/or
>> +   modify it under the terms of the GNU Lesser General Public
>> +   License as published by the Free Software Foundation; either
>> +   version 2.1 of the License, or (at your option) any later version.
>> +
>> +   The GNU C Library is distributed in the hope that it will be useful,
>> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
>> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>> +   Lesser General Public License for more details.
>> +
>> +   You should have received a copy of the GNU Lesser General Public
>> +   License along with the GNU C Library; if not, see
>> +   <http://www.gnu.org/licenses/>.  */
>> +
>> +#include <sysdep.h>
>> +
>> +/* long int [r3] __syscall_cancel_arch (int *cancelhandling [r3],
>> +					long int nr   [r4],
>> +					long int arg1 [r5],
>> +					long int arg2 [r6],
>> +					long int arg3 [r7],
>> +					long int arg4 [r8],
>> +					long int arg5 [r9],
>> +					long int arg6 [r10])  */
>> +
>> +ENTRY (__syscall_cancel_arch)
>> +
>> +	.globl __syscall_cancel_arch_start
>> +	.type  __syscall_cancel_arch_start,@function
>> +__syscall_cancel_arch_start:
>> +
>> +	/* if (*cancelhandling & CANCELED_BITMASK)
>> +	     __syscall_do_cancel()  */
>> +	lwz     r0,0(r3)
>> +	rldicl. r0,r0,62,63
>> +	beq     1f
>> +	b       __syscall_do_cancel
> 
> Should this be a Branch & Link?
> 

Right, I think ABI really mandates branch & link even for no-return calls.
I will change it.

Patch

diff --git a/sysdeps/unix/sysv/linux/powerpc/sigcontextinfo.h b/sysdeps/unix/sysv/linux/powerpc/sigcontextinfo.h
index 72381e3..3be71f7 100644
--- a/sysdeps/unix/sysv/linux/powerpc/sigcontextinfo.h
+++ b/sysdeps/unix/sysv/linux/powerpc/sigcontextinfo.h
@@ -15,7 +15,11 @@ 
    License along with the GNU C Library; if not, see
    <http://www.gnu.org/licenses/>.  */
 
+#ifndef _SIGCONTEXTINFO_H
+#define _SIGCONTEXTINFO_H
+
 #include <signal.h>
+#include <stdint.h>
 
 #define SIGCONTEXT struct sigcontext *
 #define SIGCONTEXT_EXTRA_ARGS
@@ -24,3 +28,15 @@ 
 #define GET_STACK(ctx)	((void *)((ctx)->regs->gpr[1]))
 #define CALL_SIGHANDLER(handler, signo, ctx) \
   (handler)((signo), SIGCONTEXT_EXTRA_ARGS (ctx))
+
+static inline uintptr_t
+ucontext_get_pc (const ucontext_t *uc)
+{
+#ifdef __powerpc64__
+  return uc->uc_mcontext.gp_regs[PT_NIP];
+#else
+  return uc->uc_mcontext.uc_regs->gregs[PT_NIP];
+#endif
+}
+
+#endif /* _SIGCONTEXTINFO_H  */
diff --git a/sysdeps/unix/sysv/linux/powerpc/syscall_cancel.S b/sysdeps/unix/sysv/linux/powerpc/syscall_cancel.S
new file mode 100644
index 0000000..2e56c72
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/powerpc/syscall_cancel.S
@@ -0,0 +1,64 @@ 
+/* Cancellable syscall wrapper.  Linux/powerpc version.
+   Copyright (C) 2017 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+#include <sysdep.h>
+
+/* long int [r3] __syscall_cancel_arch (int *cancelhandling [r3],
+					long int nr   [r4],
+					long int arg1 [r5],
+					long int arg2 [r6],
+					long int arg3 [r7],
+					long int arg4 [r8],
+					long int arg5 [r9],
+					long int arg6 [r10])  */
+
+ENTRY (__syscall_cancel_arch)
+
+	.globl __syscall_cancel_arch_start
+	.type  __syscall_cancel_arch_start,@function
+__syscall_cancel_arch_start:
+
+	/* if (*cancelhandling & CANCELED_BITMASK)
+	     __syscall_do_cancel()  */
+	lwz     r0,0(r3)
+	rldicl. r0,r0,62,63
+	beq     1f
+	b       __syscall_do_cancel
+	nop
+1:
+	ABORT_TRANSACTION
+	/* Issue a 6 argument syscall, the nr [r4] being the syscall
+	   number.  */
+	mr      r0,r4
+	mr      r3,r5
+	mr      r4,r6
+	mr      r5,r7
+	mr      r6,r8
+	mr      r7,r9
+	mr      r8,r10
+	sc
+
+	.globl __syscall_cancel_arch_end
+	.type  __syscall_cancel_arch_end,@function
+__syscall_cancel_arch_end:
+
+	bnslr+
+	neg	r3,r3
+	blr
+END (__syscall_cancel_arch)
+libc_hidden_def (__syscall_cancel_arch)