Re: [PATCH 2/3] Consolidate Linux sigprocmask() implementation

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

On 31/10/2017 20:58, Yury Norov wrote:
> Hi Adhemerval!
> 
> On Tue, Oct 31, 2017 at 07:19:01PM -0200, Adhemerval Zanella wrote:
>>
>>
>> On 16/10/2017 02:34, Yury Norov wrote:
>>> ia64, s390-64, sparc64, x86_64 and alpha ports has their own
>>> implementations of sigprocmask(). They all but alpha do exactly
>>> what generic sigprocmask() except the check and clear SIGCANCEL
>>> and SIGSETXID flags.
>>>
>>> In this patch, the NEED_CLEAR_SIGCANCEL_SIGSETXID option is
>>> introduced and disabled for that ports which allows to swith
>>> them to generic implementation.
>>
>> Although the manual do not state the Linux implementation detail I think
>> all supported Linux architecture should have the same semantic regarding 
>> SIGCANCEL and SIGSETXID.  GLIBC on Linux requires both signal to proper
>> implement both pthread cancellation and set*id function and having
>> different semanticsis troublesome (a conformant program on a architecture
>> that does not filter out the signals might inadvertently disable pthread
>> asynchronous cancellation, set*id synchronization or posix timers).
>>
>> Also, sigfillset removes SIGCANCEL and SIGSETXID as expected, but
>> sigaddset and sigdelset does not handle none of internal signals.  I also
>> think we should ignore internal nptl signals on sigaddset and sigdelset.
>>
>> And for this specific case I don't see adding compat symbols to keep
>> the old semantic for the related architectures the best approach.  There
>> is a canonical way to actually disable pthread cancellation and masking
>> SIGSETXID would make set*id non POSIX conformant.
>>
>> What about the following?
> 
> I suspected that sigprocmask is buggy and should be fixed as you
> suggested here. Now after your explanation I'm convinced with it. But
> your patch changes user interface to glibc which may break existing
> software.
> 
> The most conservative way to proceed with it is to leave the existing
> behavior for affected platforms as is. For software compiled against
> glibc-2.27 or newer we can use versioning to wire sigprocmask to 
> __new_sigprocmask, which would emit warning for x86 and others, and
> clear internal signals if they appear.
> 
> But I'm not familiar with nptl, and if you think that silent API
> change will not hurt users, I'm OK with your patch as is. In this
> case I would only ask you to add notes about this changes to NEWS,
> and especially about alpha as it is switched to new syscall.
> 
> Yury

I think the main problem of providing a compat symbol is besides 
interfering with both pthread cancellation and posix timers (a explicit
conformance break), not filtering out NPTL internal signals for set*id 
programs might be a security issue where the user/group id is not 
synchronized over the threads as expected by a POSIX standard.

I have opened BZ#22391 [1] to track this issue. I am also preparing
a patch set to fix this over the signal implementations.

[1] https://sourceware.org/bugzilla/show_bug.cgi?id=22391