[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20200205192756.GC1663@brightrain.aerifal.cx>
Date: Wed, 5 Feb 2020 14:27:56 -0500
From: Rich Felker <dalias@...c.org>
To: musl@...ts.openwall.com
Subject: Re: [PATCH] add support for pthread_sigqueue
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 <daniel@...lgren.se>
> 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 <signal.h>
> +#include <string.h>
> +#include <unistd.h>
> +#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?
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
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
