OS-7121 lx vfork and signal handling still racey

Reviewed by: Patrick Mooney <patrick.mooney@joyent.com> Reviewed by: Jerry Jelinek <jerry.jelinek@joyent.com> Reviewed by: Ryan Zezeski <rpz@joyent.com> Approved by: Robert Mustacchi <rm@joyent.com>
TritonDataCenter · Oct 2, 2018 · fe10e07 · fe10e07
1 parent 1f6101e
commit fe10e07
Show file tree

Hide file tree

Showing 4 changed files with 44 additions and 6 deletions.
diff --git a/usr/src/lib/brand/lx/lx_brand/common/signal.c b/usr/src/lib/brand/lx/lx_brand/common/signal.c
@@ -1450,7 +1450,7 @@ lx_call_user_handler(int sig, siginfo_t *sip, void *p)
 {
 	void (*user_handler)();
 	void (*stk_builder)();
-	struct lx_sigaction *lxsap;
+	volatile struct lx_sigaction *lxsap;
 	ucontext_t *ucp = (ucontext_t *)p;
 	size_t stksize;
 	int lx_sig;
@@ -1480,9 +1480,30 @@ lx_call_user_handler(int sig, siginfo_t *sip, void *p)
 		assert(0);
 	}
 
-	if (lxsap->lxsa_handler == SIG_DFL || lxsap->lxsa_handler == SIG_IGN)
+	while (lxsap->lxsa_handler == SIG_DFL ||
+	    lxsap->lxsa_handler == SIG_IGN) {
+		/*
+		 * This normally shouldn't be possible, but there is a window
+		 * in which a vfork()'d process can have its signal disposition
+		 * corrupted by its child.  While this window is narrowed by
+		 * blocking all signals in the brand, that leaves a (smaller)
+		 * window whereby a signal in flight is delivered before the
+		 * brand has blocked them.  To detect this case, we will spin
+		 * if our signal disposition is impossible and all signals are
+		 * blocked due to vfork() activity:  we know that the vfork()'d
+		 * child will eventually restore the signal disposition before
+		 * it unblocks signals, allowing us to proceed.
+		 */
+		if (lx_all_signals_blocked())
+			continue;
+
+		if (lxsap->lxsa_handler != SIG_DFL &&
+		    lxsap->lxsa_handler != SIG_IGN)
+			break;
+
 		lx_err_fatal("lxsa_handler set to %s?  How?!?!?",
 		    (lxsap->lxsa_handler == SIG_DFL) ? "SIG_DFL" : "SIG_IGN");
+	}
 
 #if defined(_LP64)
 	stksize = sizeof (struct lx_sigstack);
@@ -1506,7 +1527,7 @@ lx_call_user_handler(int sig, siginfo_t *sip, void *p)
 		lxsap->lxsa_handler = SIG_DFL;
 
 	lx_sigdeliver(lx_sig, sip, ucp, stksize, stk_builder, user_handler,
-	    lxsap);
+	    (struct lx_sigaction *)lxsap);
 
 	/*
 	 * We need to handle restarting system calls if requested by the
@@ -2390,3 +2411,9 @@ lx_unblock_all_signals()
 {
 	(void) syscall(SYS_brand, B_UNBLOCK_ALL_SIGS);
 }
+
+int
+lx_all_signals_blocked()
+{
+	return (syscall(SYS_brand, B_ALL_SIGS_BLOCKED));
+}
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
@@ -25,7 +25,7 @@
  */
 
 /*
- * Copyright 2017 Joyent, Inc.
+ * Copyright 2018 Joyent, Inc.
  */
 
 #ifndef _SYS_LX_H
@@ -149,6 +149,7 @@ extern void lx_stack_postfork(void);
 
 extern void lx_block_all_signals();
 extern void lx_unblock_all_signals();
+extern int lx_all_signals_blocked();
 
 /*
  * NO_UUCOPY disables calls to the uucopy* system calls to help with

diff --git a/usr/src/uts/common/brand/lx/os/lx_brand.c b/usr/src/uts/common/brand/lx/os/lx_brand.c
@@ -25,7 +25,7 @@
  */
 
 /*
- * Copyright 2017, Joyent, Inc. All rights reserved.
+ * Copyright 2018, Joyent, Inc. All rights reserved.
  */
 
 /*
@@ -1892,6 +1892,16 @@ lx_brandsys(int cmd, int64_t *rval, uintptr_t arg1, uintptr_t arg2,
 		mutex_exit(&p->p_lock);
 		return (result);
 	}
+
+	case B_ALL_SIGS_BLOCKED: {
+		uint_t result;
+
+		mutex_enter(&p->p_lock);
+		pd = ptolxproc(p);
+		result = pd->l_block_all_signals;
+		mutex_exit(&p->p_lock);
+		return (result);
+	}
 	}
 
 	return (EINVAL);

diff --git a/usr/src/uts/common/brand/lx/sys/lx_brand.h b/usr/src/uts/common/brand/lx/sys/lx_brand.h
@@ -103,7 +103,7 @@ extern "C" {
 #define	B_STORE_ARGS		137
 #define	B_GETPID		138
 #define	B_JUMP_TO_LINUX		139
-/* formerly B_SET_THUNK_PID	140 */
+#define	B_ALL_SIGS_BLOCKED	140
 #define	B_EXIT_AS_SIG		141
 /* formerly B_HELPER_WAITID	142 */
 #define	B_HELPER_CLONE		143