| Message ID | 20171016043407.1142-3-ynorov@caviumnetworks.com |
|---|---|
| State | New |
| Headers | show |
| Series | Cleanup and consolidate sigprocmask() for Linux | expand |
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? --- This patch consolidates the sigprocmask Linux syscall implementation on sysdeps/unix/sysv/linux/sigprocmask.c. The changes are: 1. For ia64, s390-64, sparc64, and x86_64 the default semantic for filter out SIGCANCEL and SIGSETXID is used. Also the Linux pthread semantic is documented in the signal chapter. 2. A new internal function to check for NPTL internal signals within a signal set is added (__nptl_has_internal_signal). It is used to simplify the default sigprocmask.c implementation. Checked on x86_64-linux-gnu. * manual/signal.texi: Add a note about internal pthread signals on Linux. * sysdeps/unix/sysv/linux/alpha/sigprocmask.c: Remove file. * sysdeps/unix/sysv/linux/ia64/sigprocmask.c: Likewise. * sysdeps/unix/sysv/linux/sparc/sparc64/sigprocmask.c: Likewise. * sysdeps/unix/sysv/linux/x86_64/sigprocmask.c: Likewise. * sysdeps/unix/sysv/linux/nptl-signals.h (__nptl_has_internal_signal): New function. * sysdeps/unix/sysv/linux/sigprocmask.c (__sigprocmask): Use __nptl_has_internal_signal and __nptl_clear_internal_signals function. Signed-off-by: Adhemerval Zanella <adhemerval.zanella@linaro.org> --- diff --git a/manual/signal.texi b/manual/signal.texi index 9577ff0..46696af 100644 --- a/manual/signal.texi +++ b/manual/signal.texi @@ -2461,6 +2461,11 @@ well. (In addition, it's not wise to put into your program an assumption that the system has no signals aside from the ones you know about.) +On Linux pthreads internally uses some signals to implement asynchronous +cancellation, effective ID synchronization for POSIX conformance, and +posix timer management. These signals are filtered out on signal set +function manipulation. + @deftypefun int sigemptyset (sigset_t *@var{set}) @standards{POSIX.1, signal.h} @safety{@prelim{}@mtsafe{}@assafe{}@acsafe{}} diff --git a/sysdeps/unix/sysv/linux/alpha/sigprocmask.c b/sysdeps/unix/sysv/linux/alpha/sigprocmask.c deleted file mode 100644 index ebec70c..0000000 --- a/sysdeps/unix/sysv/linux/alpha/sigprocmask.c +++ /dev/null @@ -1,58 +0,0 @@ -/* Copyright (C) 1993-2017 Free Software Foundation, Inc. - This file is part of the GNU C Library. - Contributed by David Mosberger (davidm@azstarnet.com). - - 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 <errno.h> -#include <sysdep.h> -#include <signal.h> - -/* When there is kernel support for more than 64 signals, we'll have to - switch to a new system call convention here. */ - -int -__sigprocmask (int how, const sigset_t *set, sigset_t *oset) -{ - unsigned long int setval; - long result; - - if (set) - setval = set->__val[0]; - else - { - setval = 0; - how = SIG_BLOCK; /* ensure blocked mask doesn't get changed */ - } - - result = INLINE_SYSCALL (osf_sigprocmask, 2, how, setval); - if (result == -1) - /* If there are ever more than 63 signals, we need to recode this - in assembler since we wouldn't be able to distinguish a mask of - all 1s from -1, but for now, we're doing just fine... */ - return result; - - if (oset) - { - oset->__val[0] = result; - result = _SIGSET_NWORDS; - while (--result > 0) - oset->__val[result] = 0; - } - return 0; -} - -libc_hidden_def (__sigprocmask) -weak_alias (__sigprocmask, sigprocmask); diff --git a/sysdeps/unix/sysv/linux/ia64/sigprocmask.c b/sysdeps/unix/sysv/linux/ia64/sigprocmask.c deleted file mode 100644 index 920c5fd..0000000 --- a/sysdeps/unix/sysv/linux/ia64/sigprocmask.c +++ /dev/null @@ -1,40 +0,0 @@ -/* Copyright (C) 1997-2017 Free Software Foundation, Inc. - This file is part of the GNU C Library. - Linux/IA64 specific sigprocmask - Written by Jes Sorensen, <Jes.Sorensen@cern.ch>, April 1999. - - 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/>. */ - -/* Linux/ia64 only has rt signals, thus we do not even want to try falling - back to the old style signals as the default Linux handler does. */ - -#include <errno.h> -#include <signal.h> -#include <unistd.h> - -#include <sysdep.h> -#include <sys/syscall.h> - -/* Get and/or change the set of blocked signals. */ -int -__sigprocmask (int how, const sigset_t *set, sigset_t *oset) -{ - - /* XXX The size argument hopefully will have to be changed to the - real size of the user-level sigset_t. */ - return INLINE_SYSCALL (rt_sigprocmask, 4, how, set, oset, _NSIG / 8); -} -libc_hidden_def (__sigprocmask) -weak_alias (__sigprocmask, sigprocmask) diff --git a/sysdeps/unix/sysv/linux/nptl-signals.h b/sysdeps/unix/sysv/linux/nptl-signals.h index f30c597..6afd3fe 100644 --- a/sysdeps/unix/sysv/linux/nptl-signals.h +++ b/sysdeps/unix/sysv/linux/nptl-signals.h @@ -33,6 +33,12 @@ #define SIGSETXID (__SIGRTMIN + 1) +static inline bool +__nptl_has_internal_signal (const sigset_t *set) +{ + return __sigismember (set, SIGCANCEL) || __sigismember (set, SIGSETXID); +} + /* Return is sig is used internally. */ static inline int __nptl_is_internal_signal (int sig) diff --git a/sysdeps/unix/sysv/linux/s390/s390-64/sigprocmask.c b/sysdeps/unix/sysv/linux/s390/s390-64/sigprocmask.c deleted file mode 100644 index a8010e7..0000000 --- a/sysdeps/unix/sysv/linux/s390/s390-64/sigprocmask.c +++ /dev/null @@ -1,38 +0,0 @@ -/* Copyright (C) 2001-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/>. */ - -/* 64 bit Linux for S/390 only has rt signals, thus we do not even want to try - falling back to the old style signals as the default Linux handler does. */ - -#include <errno.h> -#include <signal.h> -#include <unistd.h> - -#include <sysdep.h> -#include <sys/syscall.h> - -/* Get and/or change the set of blocked signals. */ -int -__sigprocmask (int how, const sigset_t *set, sigset_t *oset) -{ - - /* XXX The size argument hopefully will have to be changed to the - real size of the user-level sigset_t. */ - return INLINE_SYSCALL (rt_sigprocmask, 4, how, set, oset, _NSIG / 8); -} -libc_hidden_def (__sigprocmask) -weak_alias (__sigprocmask, sigprocmask) diff --git a/sysdeps/unix/sysv/linux/sigprocmask.c b/sysdeps/unix/sysv/linux/sigprocmask.c index e776563..d14fc5c 100644 --- a/sysdeps/unix/sysv/linux/sigprocmask.c +++ b/sysdeps/unix/sysv/linux/sigprocmask.c @@ -1,4 +1,5 @@ -/* Copyright (C) 1997-2017 Free Software Foundation, Inc. +/* Get and/or change the set of blocked signals. Linux version. + Copyright (C) 1997-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 @@ -17,34 +18,22 @@ #include <errno.h> #include <signal.h> -#include <string.h> /* Needed for string function builtin redirection. */ -#include <unistd.h> +#include <nptl-signals.h> -#include <sysdep.h> -#include <sys/syscall.h> -#include <nptl/pthreadP.h> /* SIGCANCEL, SIGSETXID */ - - -/* Get and/or change the set of blocked signals. */ int __sigprocmask (int how, const sigset_t *set, sigset_t *oset) { sigset_t local_newmask; - /* The only thing we have to make sure here is that SIGCANCEL and - SIGSETXID are not blocked. */ - if (set != NULL - && (__builtin_expect (__sigismember (set, SIGCANCEL), 0) - || __builtin_expect (__sigismember (set, SIGSETXID), 0))) + if (set != NULL && __glibc_unlikely (__nptl_has_internal_signal (set))) { local_newmask = *set; - __sigdelset (&local_newmask, SIGCANCEL); - __sigdelset (&local_newmask, SIGSETXID); + __nptl_clear_internal_signals (&local_newmask); set = &local_newmask; } - return INLINE_SYSCALL (rt_sigprocmask, 4, how, set, oset, _NSIG / 8); + return INLINE_SYSCALL_CALL (rt_sigprocmask, how, set, oset, _NSIG / 8); } libc_hidden_def (__sigprocmask) weak_alias (__sigprocmask, sigprocmask) diff --git a/sysdeps/unix/sysv/linux/sparc/sparc64/sigprocmask.c b/sysdeps/unix/sysv/linux/sparc/sparc64/sigprocmask.c deleted file mode 100644 index ef7d7fe..0000000 --- a/sysdeps/unix/sysv/linux/sparc/sparc64/sigprocmask.c +++ /dev/null @@ -1,34 +0,0 @@ -/* Copyright (C) 1997-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 <errno.h> -#include <signal.h> -#include <unistd.h> - -#include <sysdep.h> -#include <sys/syscall.h> - -/* Get and/or change the set of blocked signals. */ -int -__sigprocmask (int how, const sigset_t *set, sigset_t *oset) -{ - /* XXX The size argument hopefully will have to be changed to the - real size of the user-level sigset_t. */ - return INLINE_SYSCALL (rt_sigprocmask, 4, how, set, oset, _NSIG / 8); -} -libc_hidden_def (__sigprocmask) -weak_alias (__sigprocmask, sigprocmask) diff --git a/sysdeps/unix/sysv/linux/x86_64/sigprocmask.c b/sysdeps/unix/sysv/linux/x86_64/sigprocmask.c deleted file mode 100644 index 1610ddf..0000000 --- a/sysdeps/unix/sysv/linux/x86_64/sigprocmask.c +++ /dev/null @@ -1,39 +0,0 @@ -/* Copyright (C) 1997-2017 Free Software Foundation, Inc. - This file is part of the GNU C Library. - Written by Jes Sorensen, <Jes.Sorensen@cern.ch>, April 1999. - - 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/>. */ - -/* Linux/x86_64 only has rt signals, thus we do not even want to try falling - back to the old style signals as the default Linux handler does. */ - -#include <errno.h> -#include <signal.h> -#include <unistd.h> - -#include <sysdep.h> -#include <sys/syscall.h> - -/* Get and/or change the set of blocked signals. */ -int -__sigprocmask (int how, const sigset_t *set, sigset_t *oset) -{ - - /* XXX The size argument hopefully will have to be changed to the - real size of the user-level sigset_t. */ - return INLINE_SYSCALL (rt_sigprocmask, 4, how, set, oset, _NSIG / 8); -} -libc_hidden_def (__sigprocmask) -weak_alias (__sigprocmask, sigprocmask)
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 > --- > > This patch consolidates the sigprocmask Linux syscall implementation on > sysdeps/unix/sysv/linux/sigprocmask.c. The changes are: > > 1. For ia64, s390-64, sparc64, and x86_64 the default semantic for > filter out SIGCANCEL and SIGSETXID is used. Also the Linux pthread > semantic is documented in the signal chapter. > > 2. A new internal function to check for NPTL internal signals within a > signal set is added (__nptl_has_internal_signal). It is used to > simplify the default sigprocmask.c implementation. > > Checked on x86_64-linux-gnu. > > * manual/signal.texi: Add a note about internal pthread signals > on Linux. > * sysdeps/unix/sysv/linux/alpha/sigprocmask.c: Remove file. > * sysdeps/unix/sysv/linux/ia64/sigprocmask.c: Likewise. > * sysdeps/unix/sysv/linux/sparc/sparc64/sigprocmask.c: Likewise. > * sysdeps/unix/sysv/linux/x86_64/sigprocmask.c: Likewise. > * sysdeps/unix/sysv/linux/nptl-signals.h > (__nptl_has_internal_signal): New function. > * sysdeps/unix/sysv/linux/sigprocmask.c (__sigprocmask): > Use __nptl_has_internal_signal and __nptl_clear_internal_signals > function. > > Signed-off-by: Adhemerval Zanella <adhemerval.zanella@linaro.org> > > --- > > diff --git a/manual/signal.texi b/manual/signal.texi > index 9577ff0..46696af 100644 > --- a/manual/signal.texi > +++ b/manual/signal.texi > @@ -2461,6 +2461,11 @@ well. (In addition, it's not wise to put into your program an > assumption that the system has no signals aside from the ones you know > about.) > > +On Linux pthreads internally uses some signals to implement asynchronous > +cancellation, effective ID synchronization for POSIX conformance, and > +posix timer management. These signals are filtered out on signal set > +function manipulation. > + > @deftypefun int sigemptyset (sigset_t *@var{set}) > @standards{POSIX.1, signal.h} > @safety{@prelim{}@mtsafe{}@assafe{}@acsafe{}} > diff --git a/sysdeps/unix/sysv/linux/alpha/sigprocmask.c b/sysdeps/unix/sysv/linux/alpha/sigprocmask.c > deleted file mode 100644 > index ebec70c..0000000 > --- a/sysdeps/unix/sysv/linux/alpha/sigprocmask.c > +++ /dev/null > @@ -1,58 +0,0 @@ > -/* Copyright (C) 1993-2017 Free Software Foundation, Inc. > - This file is part of the GNU C Library. > - Contributed by David Mosberger (davidm@azstarnet.com). > - > - 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 <errno.h> > -#include <sysdep.h> > -#include <signal.h> > - > -/* When there is kernel support for more than 64 signals, we'll have to > - switch to a new system call convention here. */ > - > -int > -__sigprocmask (int how, const sigset_t *set, sigset_t *oset) > -{ > - unsigned long int setval; > - long result; > - > - if (set) > - setval = set->__val[0]; > - else > - { > - setval = 0; > - how = SIG_BLOCK; /* ensure blocked mask doesn't get changed */ > - } > - > - result = INLINE_SYSCALL (osf_sigprocmask, 2, how, setval); > - if (result == -1) > - /* If there are ever more than 63 signals, we need to recode this > - in assembler since we wouldn't be able to distinguish a mask of > - all 1s from -1, but for now, we're doing just fine... */ > - return result; > - > - if (oset) > - { > - oset->__val[0] = result; > - result = _SIGSET_NWORDS; > - while (--result > 0) > - oset->__val[result] = 0; > - } > - return 0; > -} > - > -libc_hidden_def (__sigprocmask) > -weak_alias (__sigprocmask, sigprocmask); > diff --git a/sysdeps/unix/sysv/linux/ia64/sigprocmask.c b/sysdeps/unix/sysv/linux/ia64/sigprocmask.c > deleted file mode 100644 > index 920c5fd..0000000 > --- a/sysdeps/unix/sysv/linux/ia64/sigprocmask.c > +++ /dev/null > @@ -1,40 +0,0 @@ > -/* Copyright (C) 1997-2017 Free Software Foundation, Inc. > - This file is part of the GNU C Library. > - Linux/IA64 specific sigprocmask > - Written by Jes Sorensen, <Jes.Sorensen@cern.ch>, April 1999. > - > - 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/>. */ > - > -/* Linux/ia64 only has rt signals, thus we do not even want to try falling > - back to the old style signals as the default Linux handler does. */ > - > -#include <errno.h> > -#include <signal.h> > -#include <unistd.h> > - > -#include <sysdep.h> > -#include <sys/syscall.h> > - > -/* Get and/or change the set of blocked signals. */ > -int > -__sigprocmask (int how, const sigset_t *set, sigset_t *oset) > -{ > - > - /* XXX The size argument hopefully will have to be changed to the > - real size of the user-level sigset_t. */ > - return INLINE_SYSCALL (rt_sigprocmask, 4, how, set, oset, _NSIG / 8); > -} > -libc_hidden_def (__sigprocmask) > -weak_alias (__sigprocmask, sigprocmask) > diff --git a/sysdeps/unix/sysv/linux/nptl-signals.h b/sysdeps/unix/sysv/linux/nptl-signals.h > index f30c597..6afd3fe 100644 > --- a/sysdeps/unix/sysv/linux/nptl-signals.h > +++ b/sysdeps/unix/sysv/linux/nptl-signals.h > @@ -33,6 +33,12 @@ > #define SIGSETXID (__SIGRTMIN + 1) > > > +static inline bool > +__nptl_has_internal_signal (const sigset_t *set) > +{ > + return __sigismember (set, SIGCANCEL) || __sigismember (set, SIGSETXID); > +} > + > /* Return is sig is used internally. */ > static inline int > __nptl_is_internal_signal (int sig) > diff --git a/sysdeps/unix/sysv/linux/s390/s390-64/sigprocmask.c b/sysdeps/unix/sysv/linux/s390/s390-64/sigprocmask.c > deleted file mode 100644 > index a8010e7..0000000 > --- a/sysdeps/unix/sysv/linux/s390/s390-64/sigprocmask.c > +++ /dev/null > @@ -1,38 +0,0 @@ > -/* Copyright (C) 2001-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/>. */ > - > -/* 64 bit Linux for S/390 only has rt signals, thus we do not even want to try > - falling back to the old style signals as the default Linux handler does. */ > - > -#include <errno.h> > -#include <signal.h> > -#include <unistd.h> > - > -#include <sysdep.h> > -#include <sys/syscall.h> > - > -/* Get and/or change the set of blocked signals. */ > -int > -__sigprocmask (int how, const sigset_t *set, sigset_t *oset) > -{ > - > - /* XXX The size argument hopefully will have to be changed to the > - real size of the user-level sigset_t. */ > - return INLINE_SYSCALL (rt_sigprocmask, 4, how, set, oset, _NSIG / 8); > -} > -libc_hidden_def (__sigprocmask) > -weak_alias (__sigprocmask, sigprocmask) > diff --git a/sysdeps/unix/sysv/linux/sigprocmask.c b/sysdeps/unix/sysv/linux/sigprocmask.c > index e776563..d14fc5c 100644 > --- a/sysdeps/unix/sysv/linux/sigprocmask.c > +++ b/sysdeps/unix/sysv/linux/sigprocmask.c > @@ -1,4 +1,5 @@ > -/* Copyright (C) 1997-2017 Free Software Foundation, Inc. > +/* Get and/or change the set of blocked signals. Linux version. > + Copyright (C) 1997-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 > @@ -17,34 +18,22 @@ > > #include <errno.h> > #include <signal.h> > -#include <string.h> /* Needed for string function builtin redirection. */ > -#include <unistd.h> > +#include <nptl-signals.h> > > -#include <sysdep.h> > -#include <sys/syscall.h> > > -#include <nptl/pthreadP.h> /* SIGCANCEL, SIGSETXID */ > - > - > -/* Get and/or change the set of blocked signals. */ > int > __sigprocmask (int how, const sigset_t *set, sigset_t *oset) > { > sigset_t local_newmask; > > - /* The only thing we have to make sure here is that SIGCANCEL and > - SIGSETXID are not blocked. */ > - if (set != NULL > - && (__builtin_expect (__sigismember (set, SIGCANCEL), 0) > - || __builtin_expect (__sigismember (set, SIGSETXID), 0))) > + if (set != NULL && __glibc_unlikely (__nptl_has_internal_signal (set))) > { > local_newmask = *set; > - __sigdelset (&local_newmask, SIGCANCEL); > - __sigdelset (&local_newmask, SIGSETXID); > + __nptl_clear_internal_signals (&local_newmask); > set = &local_newmask; > } > > - return INLINE_SYSCALL (rt_sigprocmask, 4, how, set, oset, _NSIG / 8); > + return INLINE_SYSCALL_CALL (rt_sigprocmask, how, set, oset, _NSIG / 8); > } > libc_hidden_def (__sigprocmask) > weak_alias (__sigprocmask, sigprocmask) > diff --git a/sysdeps/unix/sysv/linux/sparc/sparc64/sigprocmask.c b/sysdeps/unix/sysv/linux/sparc/sparc64/sigprocmask.c > deleted file mode 100644 > index ef7d7fe..0000000 > --- a/sysdeps/unix/sysv/linux/sparc/sparc64/sigprocmask.c > +++ /dev/null > @@ -1,34 +0,0 @@ > -/* Copyright (C) 1997-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 <errno.h> > -#include <signal.h> > -#include <unistd.h> > - > -#include <sysdep.h> > -#include <sys/syscall.h> > - > -/* Get and/or change the set of blocked signals. */ > -int > -__sigprocmask (int how, const sigset_t *set, sigset_t *oset) > -{ > - /* XXX The size argument hopefully will have to be changed to the > - real size of the user-level sigset_t. */ > - return INLINE_SYSCALL (rt_sigprocmask, 4, how, set, oset, _NSIG / 8); > -} > -libc_hidden_def (__sigprocmask) > -weak_alias (__sigprocmask, sigprocmask) > diff --git a/sysdeps/unix/sysv/linux/x86_64/sigprocmask.c b/sysdeps/unix/sysv/linux/x86_64/sigprocmask.c > deleted file mode 100644 > index 1610ddf..0000000 > --- a/sysdeps/unix/sysv/linux/x86_64/sigprocmask.c > +++ /dev/null > @@ -1,39 +0,0 @@ > -/* Copyright (C) 1997-2017 Free Software Foundation, Inc. > - This file is part of the GNU C Library. > - Written by Jes Sorensen, <Jes.Sorensen@cern.ch>, April 1999. > - > - 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/>. */ > - > -/* Linux/x86_64 only has rt signals, thus we do not even want to try falling > - back to the old style signals as the default Linux handler does. */ > - > -#include <errno.h> > -#include <signal.h> > -#include <unistd.h> > - > -#include <sysdep.h> > -#include <sys/syscall.h> > - > -/* Get and/or change the set of blocked signals. */ > -int > -__sigprocmask (int how, const sigset_t *set, sigset_t *oset) > -{ > - > - /* XXX The size argument hopefully will have to be changed to the > - real size of the user-level sigset_t. */ > - return INLINE_SYSCALL (rt_sigprocmask, 4, how, set, oset, _NSIG / 8); > -} > -libc_hidden_def (__sigprocmask) > -weak_alias (__sigprocmask, sigprocmask)
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
diff --git a/sysdeps/unix/sysv/linux/ia64/sigprocmask.c b/sysdeps/unix/sysv/linux/ia64/sigprocmask.c index 920c5fd052..9e95e52d58 100644 --- a/sysdeps/unix/sysv/linux/ia64/sigprocmask.c +++ b/sysdeps/unix/sysv/linux/ia64/sigprocmask.c @@ -17,24 +17,6 @@ License along with the GNU C Library; if not, see <http://www.gnu.org/licenses/>. */ -/* Linux/ia64 only has rt signals, thus we do not even want to try falling - back to the old style signals as the default Linux handler does. */ +#define NEED_CLEAR_SIGCANCEL_SIGSETXID 0 -#include <errno.h> -#include <signal.h> -#include <unistd.h> - -#include <sysdep.h> -#include <sys/syscall.h> - -/* Get and/or change the set of blocked signals. */ -int -__sigprocmask (int how, const sigset_t *set, sigset_t *oset) -{ - - /* XXX The size argument hopefully will have to be changed to the - real size of the user-level sigset_t. */ - return INLINE_SYSCALL (rt_sigprocmask, 4, how, set, oset, _NSIG / 8); -} -libc_hidden_def (__sigprocmask) -weak_alias (__sigprocmask, sigprocmask) +#include <sysdeps/unix/sysv/linux/sigprocmask.c> diff --git a/sysdeps/unix/sysv/linux/s390/s390-64/sigprocmask.c b/sysdeps/unix/sysv/linux/s390/s390-64/sigprocmask.c index a8010e7f14..227b512910 100644 --- a/sysdeps/unix/sysv/linux/s390/s390-64/sigprocmask.c +++ b/sysdeps/unix/sysv/linux/s390/s390-64/sigprocmask.c @@ -15,24 +15,6 @@ License along with the GNU C Library; if not, see <http://www.gnu.org/licenses/>. */ -/* 64 bit Linux for S/390 only has rt signals, thus we do not even want to try - falling back to the old style signals as the default Linux handler does. */ +#define NEED_CLEAR_SIGCANCEL_SIGSETXID 0 -#include <errno.h> -#include <signal.h> -#include <unistd.h> - -#include <sysdep.h> -#include <sys/syscall.h> - -/* Get and/or change the set of blocked signals. */ -int -__sigprocmask (int how, const sigset_t *set, sigset_t *oset) -{ - - /* XXX The size argument hopefully will have to be changed to the - real size of the user-level sigset_t. */ - return INLINE_SYSCALL (rt_sigprocmask, 4, how, set, oset, _NSIG / 8); -} -libc_hidden_def (__sigprocmask) -weak_alias (__sigprocmask, sigprocmask) +#include <sysdeps/unix/sysv/linux/sigprocmask.c> diff --git a/sysdeps/unix/sysv/linux/sigprocmask.c b/sysdeps/unix/sysv/linux/sigprocmask.c index e776563336..94d4d0945f 100644 --- a/sysdeps/unix/sysv/linux/sigprocmask.c +++ b/sysdeps/unix/sysv/linux/sigprocmask.c @@ -17,19 +17,25 @@ #include <errno.h> #include <signal.h> -#include <string.h> /* Needed for string function builtin redirection. */ #include <unistd.h> #include <sysdep.h> #include <sys/syscall.h> -#include <nptl/pthreadP.h> /* SIGCANCEL, SIGSETXID */ +#ifndef NEED_CLEAR_SIGCANCEL_SIGSETXID +#define NEED_CLEAR_SIGCANCEL_SIGSETXID 1 +#endif +#if NEED_CLEAR_SIGCANCEL_SIGSETXID +#include <nptl/pthreadP.h> +#include <string.h> +#endif /* Get and/or change the set of blocked signals. */ int __sigprocmask (int how, const sigset_t *set, sigset_t *oset) { +#if NEED_CLEAR_SIGCANCEL_SIGSETXID sigset_t local_newmask; /* The only thing we have to make sure here is that SIGCANCEL and @@ -43,6 +49,7 @@ __sigprocmask (int how, const sigset_t *set, sigset_t *oset) __sigdelset (&local_newmask, SIGSETXID); set = &local_newmask; } +#endif return INLINE_SYSCALL (rt_sigprocmask, 4, how, set, oset, _NSIG / 8); } diff --git a/sysdeps/unix/sysv/linux/sparc/sparc64/sigprocmask.c b/sysdeps/unix/sysv/linux/sparc/sparc64/sigprocmask.c index ef7d7feb49..0db24d6fce 100644 --- a/sysdeps/unix/sysv/linux/sparc/sparc64/sigprocmask.c +++ b/sysdeps/unix/sysv/linux/sparc/sparc64/sigprocmask.c @@ -15,20 +15,6 @@ License along with the GNU C Library; if not, see <http://www.gnu.org/licenses/>. */ -#include <errno.h> -#include <signal.h> -#include <unistd.h> +#define NEED_CLEAR_SIGCANCEL_SIGSETXID 0 -#include <sysdep.h> -#include <sys/syscall.h> - -/* Get and/or change the set of blocked signals. */ -int -__sigprocmask (int how, const sigset_t *set, sigset_t *oset) -{ - /* XXX The size argument hopefully will have to be changed to the - real size of the user-level sigset_t. */ - return INLINE_SYSCALL (rt_sigprocmask, 4, how, set, oset, _NSIG / 8); -} -libc_hidden_def (__sigprocmask) -weak_alias (__sigprocmask, sigprocmask) +#include <sysdeps/unix/sysv/linux/sigprocmask.c> diff --git a/sysdeps/unix/sysv/linux/x86_64/sigprocmask.c b/sysdeps/unix/sysv/linux/x86_64/sigprocmask.c index 1610ddf47f..d3179ebea5 100644 --- a/sysdeps/unix/sysv/linux/x86_64/sigprocmask.c +++ b/sysdeps/unix/sysv/linux/x86_64/sigprocmask.c @@ -16,24 +16,6 @@ License along with the GNU C Library; if not, see <http://www.gnu.org/licenses/>. */ -/* Linux/x86_64 only has rt signals, thus we do not even want to try falling - back to the old style signals as the default Linux handler does. */ +#define NEED_CLEAR_SIGCANCEL_SIGSETXID 0 -#include <errno.h> -#include <signal.h> -#include <unistd.h> - -#include <sysdep.h> -#include <sys/syscall.h> - -/* Get and/or change the set of blocked signals. */ -int -__sigprocmask (int how, const sigset_t *set, sigset_t *oset) -{ - - /* XXX The size argument hopefully will have to be changed to the - real size of the user-level sigset_t. */ - return INLINE_SYSCALL (rt_sigprocmask, 4, how, set, oset, _NSIG / 8); -} -libc_hidden_def (__sigprocmask) -weak_alias (__sigprocmask, sigprocmask) +#include <sysdeps/unix/sysv/linux/sigprocmask.c>