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

[Adhemerval Zanella <adhemerval dot zanella at linaro dot org>]

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.