[PATCH] Re: With FreeBSD 8.0 it seems like you can't block pending but undelivered signals (Bug #9550)

Jeremy Allison jra at samba.org
Mon Jan 14 16:42:54 MST 2013


On Mon, Jan 14, 2013 at 12:08:53PM -0800, Jeremy Allison wrote:
> On Sun, Jan 13, 2013 at 09:11:49AM -0800, Richard Sharpe wrote:
> > On Fri, Jan 11, 2013 at 5:24 PM, Jeremy Allison <jra at samba.org> wrote:
> > > On Wed, Jan 09, 2013 at 10:04:09AM -0800, Richard Sharpe wrote:
> > >> On Wed, Jan 9, 2013 at 8:59 AM, Jeremy Allison <jra at samba.org> wrote:
> > >> > On Wed, Jan 09, 2013 at 06:28:20AM -0800, Richard Sharpe wrote:
> > >> >
> > >> >
> > >> > Cool ! Let's work on getting a portable tested fix into tevent !
> > >>
> > >> OK, so we could test for the existence of ucontext_t as well as
> > >> SA_SIGINFO and only allow such signal handling if both exist.
> > >>
> > >> Ie, HAVE_UCONTEXT_T
> > >>
> > >> Linux has it, as does FreeBSD and I am told that Solaris has it.
> > >>
> > >> I wonder what other versions of UNIX have?
> > >
> > > Once we detect HAVE_UCONTEXT_T in tevent then I think
> > > this is the correct patch.
> > 
> > These might be the correct set of configure checks. My only concern is
> > that the AC_CHECK_HEADERS line I have used ends up defining both
> > HAVE_SYS_UCONTEXT_H and HAVE_UCONTEXT_T.
> 
> Actually I think that's perfect - maybe I should make
> my code depedent on both anyway.

Patchset that applies to master.

Please review and push if happy with it !

Jeremy.
-------------- next part --------------
>From 9d79da399805befb3fcede45e6ee378ac4d4be0a Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Mon, 14 Jan 2013 15:06:12 -0800
Subject: [PATCH 1/5] Add missing check for sys/wait.h

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 lib/replace/libreplace.m4 |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/lib/replace/libreplace.m4 b/lib/replace/libreplace.m4
index ac2af27..1ff4361 100644
--- a/lib/replace/libreplace.m4
+++ b/lib/replace/libreplace.m4
@@ -68,7 +68,7 @@ AC_FUNC_MEMCMP
 AC_CHECK_FUNCS([pipe strftime srandom random srand rand usleep setbuffer lstat getpgrp utime utimes])
 
 AC_CHECK_HEADERS(stdbool.h stdint.h sys/select.h)
-AC_CHECK_HEADERS(setjmp.h utime.h)
+AC_CHECK_HEADERS(setjmp.h utime.h sys/wait.h)
 
 LIBREPLACE_PROVIDE_HEADER([stdint.h])
 LIBREPLACE_PROVIDE_HEADER([stdbool.h])
-- 
1.7.7.3


>From 1a3afb521af044f3ed1d6ed33c46ecc0e5dad934 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Mon, 14 Jan 2013 15:21:12 -0800
Subject: [PATCH 2/5] Add ucontext configure waf checks.

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 lib/replace/wscript |   11 ++++++++++-
 1 files changed, 10 insertions(+), 1 deletions(-)

diff --git a/lib/replace/wscript b/lib/replace/wscript
index 296dae9..d465b48 100644
--- a/lib/replace/wscript
+++ b/lib/replace/wscript
@@ -79,7 +79,7 @@ struct foo bar = { .y = 'X', .x = 1 };
     conf.CHECK_HEADERS('sys/id.h sys/ioctl.h sys/ipc.h sys/mman.h sys/mode.h sys/ndir.h sys/priv.h')
     conf.CHECK_HEADERS('sys/resource.h sys/security.h sys/shm.h sys/statfs.h sys/statvfs.h sys/termio.h')
     conf.CHECK_HEADERS('sys/vfs.h sys/xattr.h termio.h termios.h sys/file.h')
-    conf.CHECK_HEADERS('sys/wait.h sys/stat.h malloc.h grp.h')
+    conf.CHECK_HEADERS('sys/ucontext.h sys/wait.h sys/stat.h malloc.h grp.h')
     conf.CHECK_HEADERS('sys/select.h setjmp.h utime.h sys/syslog.h syslog.h')
     conf.CHECK_HEADERS('stdarg.h vararg.h sys/mount.h mntent.h')
     conf.CHECK_HEADERS('stropts.h unix.h string.h strings.h sys/param.h limits.h')
@@ -192,6 +192,15 @@ struct foo bar = { .y = 'X', .x = 1 };
                     lib='nsl socket',
                     headers='sys/socket.h netdb.h netinet/in.h')
 
+    if conf.CONFIG_SET('HAVE_SYS_UCONTEXT_H') and conf.CONFIG_SET('HAVE_SIGNAL_H'):
+        conf.CHECK_CODE('''
+                       ucontext_t uc;
+                       sigaddset(&uc.uc_sigmask, SIGUSR1);
+                       ''',
+                       'HAVE_UCONTEXT_T',
+                       msg="Checking whether we have ucontext_t",
+                       headers='signal.h sys/ucontext.h')
+
     # these may be builtins, so we need the link=False strategy
     conf.CHECK_FUNCS('strdup memmem printf memset memcpy memmove strcpy strncpy bzero', link=False)
 
-- 
1.7.7.3


>From 821dba0b097fa61a1ffa5ce4e7e9edab80f46d66 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Mon, 14 Jan 2013 15:21:35 -0800
Subject: [PATCH 3/5] Add ucontext configure autoconf checks.

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 lib/replace/libreplace.m4 |   13 +++++++++++++
 1 files changed, 13 insertions(+), 0 deletions(-)

diff --git a/lib/replace/libreplace.m4 b/lib/replace/libreplace.m4
index 1ff4361..ea095aa 100644
--- a/lib/replace/libreplace.m4
+++ b/lib/replace/libreplace.m4
@@ -126,6 +126,7 @@ AC_CHECK_HEADERS(unix.h)
 AC_CHECK_HEADERS(malloc.h)
 AC_CHECK_HEADERS(syscall.h)
 AC_CHECK_HEADERS(sys/syscall.h)
+AC_CHECK_HEADERS(sys/ucontext.h)
 
 AC_CHECK_FUNCS(syscall setuid seteuid setreuid setresuid setgid setegid setregid setresgid setgroups)
 AC_CHECK_FUNCS(chroot bzero strerror strerror_r memalign posix_memalign getpagesize)
@@ -416,6 +417,18 @@ if test x"$libreplace_cv_struct_timespec" = x"yes"; then
    AC_DEFINE(HAVE_STRUCT_TIMESPEC,1,[Whether we have struct timespec])
 fi
 
+AC_CACHE_CHECK([for ucontext_t type],libreplace_cv_ucontext_t, [
+    AC_TRY_COMPILE([
+#include <signal.h>
+#if HAVE_SYS_UCONTEXT_H
+#include <sys/ucontext.h>
+# endif
+],[ucontext_t uc; sigaddset(&uc.uc_sigmask, SIGUSR1);],
+	libreplace_cv_ucontext_t=yes,libreplace_cv_ucontext_t=no)])
+if test x"$libreplace_cv_ucontext_t" = x"yes"; then
+   AC_DEFINE(HAVE_UCONTEXT_T,1,[Whether we have ucontext_t])
+fi
+
 AC_CHECK_FUNCS([printf memset memcpy],,[AC_MSG_ERROR([Required function not found])])
 
 echo "LIBREPLACE_BROKEN_CHECKS: END"
-- 
1.7.7.3


>From e6c668a900d12b01e9982fc3d982da59f25ddd52 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Mon, 14 Jan 2013 15:21:52 -0800
Subject: [PATCH 4/5] Include sys/ucontext.h if available.

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 lib/replace/system/wait.h |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/lib/replace/system/wait.h b/lib/replace/system/wait.h
index f0c3bdc..146c61a 100644
--- a/lib/replace/system/wait.h
+++ b/lib/replace/system/wait.h
@@ -40,6 +40,10 @@
 #include <setjmp.h>
 #endif
 
+#ifdef HAVE_SYS_UCONTEXT_H
+#include <sys/ucontext.h>
+#endif
+
 #if !defined(HAVE_SIG_ATOMIC_T_TYPE)
 typedef int sig_atomic_t;
 #endif
-- 
1.7.7.3


>From 1494ef568e798752b92f76cd2b037a30d3aff6eb Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Mon, 14 Jan 2013 15:22:11 -0800
Subject: [PATCH 5/5] Fix bug 9550 - sigprocmask does not work on FreeBSD to
 stop further signals in a signal handler

Mask off signals the correct way from the signal handler.

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 lib/tevent/tevent_signal.c |   29 +++++++++++++++++++++++++++++
 1 files changed, 29 insertions(+), 0 deletions(-)

diff --git a/lib/tevent/tevent_signal.c b/lib/tevent/tevent_signal.c
index 77ef7b0..4be54f2 100644
--- a/lib/tevent/tevent_signal.c
+++ b/lib/tevent/tevent_signal.c
@@ -121,10 +121,39 @@ static void tevent_common_signal_handler_info(int signum, siginfo_t *info,
 	if (count+1 == TEVENT_SA_INFO_QUEUE_COUNT) {
 		/* we've filled the info array - block this signal until
 		   these ones are delivered */
+#ifdef HAVE_UCONTEXT_T
+		/*
+		 * This is the only way for this to work.
+		 * By default signum is blocked inside this
+		 * signal handler using a temporary mask,
+		 * but what we really need to do now is
+		 * block it in the callers mask, so it
+		 * stays blocked when the temporary signal
+		 * handler mask is replaced when we return
+		 * from here. The callers mask can be found
+		 * in the ucontext_t passed in as the
+		 * void *uctx argument.
+		 */
+		ucontext_t *ucp = (ucontext_t *)uctx;
+		sigaddset(&ucp->uc_sigmask, signum);
+#else
+		/*
+		 * WARNING !!! WARNING !!!!
+		 *
+		 * This code doesn't work.
+		 * By default signum is blocked inside this
+		 * signal handler, but calling sigprocmask
+		 * modifies the temporary signal mask being
+		 * used *inside* this hander, which will be
+		 * replaced by the callers signal mask once
+		 * we return from here. See Samba
+		 * bug #9550 for details.
+		 */
 		sigset_t set;
 		sigemptyset(&set);
 		sigaddset(&set, signum);
 		sigprocmask(SIG_BLOCK, &set, NULL);
+#endif
 		TEVENT_SIG_INCREMENT(sig_state->sig_blocked[signum]);
 	}
 }
-- 
1.7.7.3



More information about the samba-technical mailing list