OS-4470 lxbrand unblocking signals in new threads must be atomic

Reviewed by: Robert Mustacchi <rm@joyent.com>

6 files changed, 68 insertions, 54 deletions

diff --git a/usr/src/lib/brand/lx/lx_brand/common/clone.c b/usr/src/lib/brand/lx/lx_brand/common/clone.c
index 69a5e55a55..5ed01bae6e 100644
--- a/usr/src/lib/brand/lx/lx_brand/common/clone.c
+++ b/usr/src/lib/brand/lx/lx_brand/common/clone.c
@@ -50,6 +50,7 @@
 #include <sys/lx_thread.h>
 #include <sys/fork.h>
 #include <sys/mman.h>
+#include <sys/debug.h>
 #include <lx_syscall.h>
 
 
@@ -91,8 +92,7 @@ struct clone_state {
 	void 		*c_ptidp;
 	struct lx_desc	*c_ldtinfo;	/* thread-specific segment */
 	void		*c_ctidp;
-	ucontext_t	c_uc;		/* original register state */
-	sigset_t	c_sigmask;	/* signal mask */
+	ucontext_t	c_uc;		/* original register state/sigmask */
 	lx_affmask_t	c_affmask;	/* CPU affinity mask */
 	volatile int	*c_clone_res;	/* pid/error returned to cloner */
 	int		c_ptrace_event;	/* ptrace(2) event for child stop */
@@ -239,13 +239,6 @@ clone_start(void *arg)
 	 */
 	lx_install_stack(cs->c_ntv_stk, cs->c_ntv_stk_sz, lxtsd);
 
-	if (sigprocmask(SIG_SETMASK, &cs->c_sigmask, NULL) < 0) {
-		*(cs->c_clone_res) = -errno;
-
-		lx_err_fatal("Unable to release held signals for child "
-		    "thread: %s", strerror(errno));
-	}
-
 	/*
 	 * Let the parent know that the clone has (effectively) been
 	 * completed.
@@ -253,10 +246,11 @@ clone_start(void *arg)
 	*(cs->c_clone_res) = rval;
 
 	/*
-	 * We want to load the general registers from this context, and
-	 * switch to the BRAND stack.
+	 * We want to load the general registers from this context, restore the
+	 * original signal mask, and switch to the BRAND stack.  The original
+	 * signal mask was saved to the context by lx_clone().
 	 */
-	cs->c_uc.uc_flags = UC_CPU;
+	cs->c_uc.uc_flags = UC_CPU | UC_SIGMASK;
 	cs->c_uc.uc_brand_data[0] = (void *)LX_UC_STACK_BRAND;
 
 	/*
@@ -281,16 +275,9 @@ clone_start(void *arg)
 	lx_ptrace_stop_if_option(cs->c_ptrace_event, B_TRUE, 0, &cs->c_uc);
 
 	/*
-	 * Jump to the Linux process.  The system call must not return.
+	 * Jump to the Linux process.  This call cannot return.
 	 */
-	if (syscall(SYS_brand, B_JUMP_TO_LINUX, &cs->c_uc) == -1) {
-		lx_err_fatal("B_JUMP_TO_LINUX failed: %s",
-		    strerror(errno));
-	}
-	abort();
-
-	/*NOTREACHED*/
-	return (NULL);
+	lx_jump_to_linux(&cs->c_uc);
 }
 
 /*
@@ -666,19 +653,18 @@ lx_clone(uintptr_t p1, uintptr_t p2, uintptr_t p3, uintptr_t p4,
 
 	clone_res = 0;
 
-	(void) sigfillset(&sigmask);
-
 	/*
 	 * Block all signals because the thread we create won't be able to
 	 * properly handle them until it's fully set up.
 	 */
+	VERIFY0(sigfillset(&sigmask));
 	if (sigprocmask(SIG_BLOCK, &sigmask, &osigmask) < 0) {
 		lx_debug("lx_clone sigprocmask() failed: %s", strerror(errno));
 		free(cs->c_lx_tsd);
 		free(cs);
 		return (-errno);
 	}
-	cs->c_sigmask = osigmask;
+	cs->c_uc.uc_sigmask = osigmask;
 
 	/*
 	 * Allocate the native stack for this new thread now, so that we
diff --git a/usr/src/lib/brand/lx/lx_brand/common/lx_brand.c b/usr/src/lib/brand/lx/lx_brand/common/lx_brand.c
index cdc556459e..932eb50a14 100644
--- a/usr/src/lib/brand/lx/lx_brand/common/lx_brand.c
+++ b/usr/src/lib/brand/lx/lx_brand/common/lx_brand.c
@@ -498,6 +498,27 @@ lx_init_tsd(lx_tsd_t *lxtsd)
 	}
 }
 
+void
+lx_jump_to_linux(ucontext_t *ucp)
+{
+	extern void setcontext_sigmask(ucontext_t *);
+
+	/*
+	 * Call into this private libc interface to allow us to use only the
+	 * signal mask handling part of a regular setcontext() operation.
+	 */
+	setcontext_sigmask(ucp);
+
+	if (syscall(SYS_brand, B_JUMP_TO_LINUX, ucp) != 0) {
+		lx_err_fatal("B_JUMP_TO_LINUX failed: %s", strerror(errno));
+	}
+
+	/*
+	 * This system call should not return.
+	 */
+	abort();
+}
+
 static void
 lx_start(uintptr_t sp, uintptr_t entry)
 {
@@ -530,15 +551,7 @@ lx_start(uintptr_t sp, uintptr_t entry)
 #endif
 
 	lx_debug("starting Linux program sp %p ldentry %p", sp, entry);
-
-	/*
-	 * This system call should not return.
-	 */
-	if (syscall(SYS_brand, B_JUMP_TO_LINUX, &jump_uc) == -1) {
-		lx_err_fatal("B_JUMP_TO_LINUX failed: %s",
-		    strerror(errno));
-	}
-	abort();
+	lx_jump_to_linux(&jump_uc);
 }
 
 /*ARGSUSED*/
diff --git a/usr/src/lib/brand/lx/lx_brand/common/signal.c b/usr/src/lib/brand/lx/lx_brand/common/signal.c
index 92bd6cfeb4..4a9817d130 100644
--- a/usr/src/lib/brand/lx/lx_brand/common/signal.c
+++ b/usr/src/lib/brand/lx/lx_brand/common/signal.c
@@ -1557,9 +1557,9 @@ lx_sigdeliver(int lx_sig, siginfo_t *sip, ucontext_t *ucp, size_t stacksz,
 	 * getcontext() stores the register state:
 	 */
 	volatile boolean_t signal_delivered = B_FALSE;
-	volatile uintptr_t lxfp;
-	volatile uintptr_t old_tsd_sp;
-	volatile int newstack;
+	volatile uintptr_t lxfp = 0;
+	volatile uintptr_t old_tsd_sp = 0;
+	volatile int newstack = 0;
 
 	/*
 	 * This function involves modifying the Linux process stack for this
@@ -1805,13 +1805,11 @@ lx_sigdeliver(int lx_sig, siginfo_t *sip, ucontext_t *ucp, size_t stacksz,
 		LX_REG(&jump_uc, REG_RDX) = hargs[2];
 #endif
 
-		if (syscall(SYS_brand, B_JUMP_TO_LINUX, &jump_uc) == -1) {
-			lx_err_fatal("B_JUMP_TO_LINUX failed: %s",
-			    strerror(errno));
-		}
+		lx_jump_to_linux(&jump_uc);
 	}
 
 	assert(0);
+	abort();
 
 after_signal_handler:
 	/*
diff --git a/usr/src/lib/brand/lx/lx_brand/sys/lx_misc.h b/usr/src/lib/brand/lx/lx_brand/sys/lx_misc.h
index 05a38a0788..3cd97d69c4 100644
--- a/usr/src/lib/brand/lx/lx_brand/sys/lx_misc.h
+++ b/usr/src/lib/brand/lx/lx_brand/sys/lx_misc.h
@@ -154,6 +154,8 @@ extern ucontext_t *lx_syscall_regs(void);
 extern uintptr_t lx_find_brand_sp(void);
 extern const ucontext_t *lx_find_brand_uc(void);
 
+extern void lx_jump_to_linux(ucontext_t *) __NORETURN;
+
 extern char *lx_fd_to_path(int fd, char *buf, int buf_size);
 extern int lx_lpid_to_spair(pid_t, pid_t *, lwpid_t *);
 extern int lx_lpid_to_spid(pid_t, pid_t *);
diff --git a/usr/src/lib/libc/port/mapfile-vers b/usr/src/lib/libc/port/mapfile-vers
index 296e49fe3b..6d2a77e1df 100644
--- a/usr/src/lib/libc/port/mapfile-vers
+++ b/usr/src/lib/libc/port/mapfile-vers
@@ -2973,6 +2973,7 @@ $endif
 	_semctl64;
 	set_escaped_context_cleanup;
 	set_setcontext_enforcement;
+	setcontext_sigmask;
 	_setbufend;
 	__set_errno;
 	setprojrctl;
diff --git a/usr/src/lib/libc/port/threads/sigaction.c b/usr/src/lib/libc/port/threads/sigaction.c
index 09be90e54f..ada1afd523 100644
--- a/usr/src/lib/libc/port/threads/sigaction.c
+++ b/usr/src/lib/libc/port/threads/sigaction.c
@@ -540,6 +540,33 @@ set_setcontext_enforcement(int on)
 	setcontext_enforcement = on;
 }
 
+/*
+ * The LX brand emulation library implements an operation that is analogous to
+ * setcontext(), but takes a different path in to the kernel.  So that it can
+ * correctly restore a signal mask, we expose just the signal mask handling
+ * part of the regular setcontext() routine as a private interface.
+ */
+void
+setcontext_sigmask(ucontext_t *ucp)
+{
+	ulwp_t *self = curthread;
+
+	if (ucp->uc_flags & UC_SIGMASK) {
+		block_all_signals(self);
+		delete_reserved_signals(&ucp->uc_sigmask);
+		self->ul_sigmask = ucp->uc_sigmask;
+		if (self->ul_cursig) {
+			/*
+			 * We have a deferred signal present.
+			 * The signal mask will be set when the
+			 * signal is taken in take_deferred_signal().
+			 */
+			ASSERT(self->ul_critical + self->ul_sigdefer != 0);
+			ucp->uc_flags &= ~UC_SIGMASK;
+		}
+	}
+}
+
 #pragma weak _setcontext = setcontext
 int
 setcontext(const ucontext_t *ucp)
@@ -560,20 +587,7 @@ setcontext(const ucontext_t *ucp)
 	/*
 	 * Restore previous signal mask and context link.
 	 */
-	if (uc.uc_flags & UC_SIGMASK) {
-		block_all_signals(self);
-		delete_reserved_signals(&uc.uc_sigmask);
-		self->ul_sigmask = uc.uc_sigmask;
-		if (self->ul_cursig) {
-			/*
-			 * We have a deferred signal present.
-			 * The signal mask will be set when the
-			 * signal is taken in take_deferred_signal().
-			 */
-			ASSERT(self->ul_critical + self->ul_sigdefer != 0);
-			uc.uc_flags &= ~UC_SIGMASK;
-		}
-	}
+	setcontext_sigmask(&uc);
 	self->ul_siglink = uc.uc_link;
 
 	/*