From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on inbox.vuxu.org X-Spam-Level: X-Spam-Status: No, score=-3.0 required=5.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,RCVD_IN_DNSWL_MED,RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL autolearn=ham autolearn_force=no version=3.4.2 Received: from mother.openwall.net (mother.openwall.net [195.42.179.200]) by inbox.vuxu.org (OpenSMTPD) with SMTP id 43b6e320 for ; Wed, 5 Feb 2020 23:08:57 +0000 (UTC) Received: (qmail 3904 invoked by uid 550); 5 Feb 2020 23:08:55 -0000 Mailing-List: contact musl-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-ID: Reply-To: musl@lists.openwall.com Received: (qmail 3886 invoked from network); 5 Feb 2020 23:08:55 -0000 X-Halon-ID: 7440ee61-486c-11ea-802b-00505699742c Authenticated-user: d3770 Message-ID: <47e7b9bfe2f0f96809cc37cab5eed0ef4084557e.camel@fahlgren.se> From: Daniel Fahlgren To: musl@lists.openwall.com Date: Thu, 06 Feb 2020 00:08:32 +0100 In-Reply-To: <20200205192756.GC1663@brightrain.aerifal.cx> References: <294c4c13f86c6f9ea3593309458bcb75a1d5d9e8.camel@fahlgren.se> <20200205192756.GC1663@brightrain.aerifal.cx> Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.28.5-0ubuntu0.18.04.1 Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: Re: [musl] [PATCH] add support for pthread_sigqueue On Wed, 2020-02-05 at 14:27 -0500, Rich Felker wrote: > On Wed, Feb 05, 2020 at 03:12:55PM +0100, Daniel Fahlgren wrote: > > Hi, > > > > This patch adds the function pthread_sigqueue. Since this is my > > first > > patch to musl I probably have missed something. Any feedback? > > > > Best regards, > > Daniel Fahlgren > > From 2e3e423b4d3d62fec3525c2e09fc9daf35fbe885 Mon Sep 17 00:00:00 > > 2001 > > From: Daniel Fahlgren > > Date: Wed, 5 Feb 2020 13:24:43 +0100 > > Subject: [PATCH] Add pthread_sigqueue > > > > This makes it possible to queue a signal and data to a thread > > --- > > include/signal.h | 1 + > > src/thread/pthread_sigqueue.c | 24 ++++++++++++++++++++++++ > > 2 files changed, 25 insertions(+) > > create mode 100644 src/thread/pthread_sigqueue.c > > > > diff --git a/include/signal.h b/include/signal.h > > index fbdf667b..8bb7d1b4 100644 > > --- a/include/signal.h > > +++ b/include/signal.h > > @@ -214,6 +214,7 @@ int sigqueue(pid_t, int, union sigval); > > > > int pthread_sigmask(int, const sigset_t *__restrict, sigset_t > > *__restrict); > > int pthread_kill(pthread_t, int); > > +int pthread_sigqueue(pthread_t, int, union sigval); > > > > void psiginfo(const siginfo_t *, const char *); > > void psignal(int, const char *); > > diff --git a/src/thread/pthread_sigqueue.c > > b/src/thread/pthread_sigqueue.c > > new file mode 100644 > > index 00000000..8de8d49a > > --- /dev/null > > +++ b/src/thread/pthread_sigqueue.c > > @@ -0,0 +1,23 @@ > > +#include > > +#include > > +#include > > +#include "pthread_impl.h" > > +#include "lock.h" > > + > > +int pthread_sigqueue(pthread_t t, int sig, const union sigval > > value) > > +{ > > + siginfo_t si; > > + sigset_t set; > > + int r; > > + memset(&si, 0, sizeof si); > > + si.si_signo = sig; > > + si.si_code = SI_QUEUE; > > + si.si_value = value; > > + si.si_uid = getuid(); > > + si.si_pid = getpid(); > > + LOCK(t->killlock); > > + r = t->tid ? -__syscall(SYS_rt_tgsigqueueinfo, si.si_pid, t- > > >tid, sig, &si) > > + : (sig+0U >= _NSIG ? EINVAL : 0); > > + UNLOCK(t->killlock); > > + return r; > > +} > > -- > > 2.17.1 > > > > Is the naming pthread_sigqueue (vs pthread_sigqueue_np) intentional? > Normally pthread functions that are not part of the standard are > introduced as _np (non-portable) and promoted later if accepted as > standards since the pthread_* namespace is reserved for future > directions of the standard. Does glibc have this function with the > same signature already? Yes, I noticed this function was missing when trying to switch some of the code base at work from uClibc-ng to musl. According to the changelog this function has been part of glibc since 2009. > I also notice that there's a fundamental race between getting the > uid, > pid, and tid and sending the signal that would normally need to be > mitigated by blocking signals for the duration of the operation. > However, I don't think we need to care about a fork interrupting it; > if that happens, the t argument is invalid and thus the call has > undefined behavior. This leaves concurrent change of uid as a > consideration. The normal sigqueue is not doing anything special > about > that, but perhaps it should; I haven't thought enough about it yet to > have an opinion, so I'm just raising the issue for consideration at > this point. > > With that said, this mostly looks good. > > Rich I'm not sure it even is possible to do anything special. The process would have to block from getuid() until the signal is processed. If a different thread is changing the uid it doesn't really matter if that is done between getting the uid and doing the syscall, or just after the syscall has returned. As long as the change is done before the receiving thread has processed the signal the effect should be the same, or? Daniel