[PATCH 1/2] kernel/cobalt: Address SIGSHADOW_ACTION_HARDEN and self-hardening race condition

From: Jan Kiszka <[email protected]>

When a remote thread is suspended, xnthread_suspend checks if it is
currently relaxed and then calls it into hardened state via SIGSHADOW.
However, this signal may race with the thread entering primary mode on
its own, e.g. via a syscall. In that case, the thread will be kicked out
of its suspension state by the pending signal, and the migrate syscall
it then executes will not perform the expected suspension anymore.

Plug this race by introducing two thread state flags, one for pending
XNSUSP and another one for XNHELD. After having performed the requested
hardening on SIGSHADOW_ACTION_HARDEN, we are now testing for those
pending suspension states and replay them via self-suspension as needed.
If a resume took place in the meantime, the state flags were cleared
again, and nothing will happen after the migration step.

Reported-by: Richard Weinberger <[email protected]>
Signed-off-by: Jan Kiszka <[email protected]>
---

Didn't check yet if EVL solves this topic differently or is exposed to 
similar race. But given that Richard found the pattern even in Xenomai 
2, it's likely worth to check EVL carefully.

 include/cobalt/uapi/kernel/thread.h |  2 ++
 kernel/cobalt/posix/signal.c        |  1 +
 kernel/cobalt/posix/syscall.c       | 26 +++++++++++++++++++++++++-
 kernel/cobalt/sched-sporadic.c      |  5 +++--
 kernel/cobalt/thread.c              |  7 ++++++-
 5 files changed, 37 insertions(+), 4 deletions(-)

diff --git a/include/cobalt/uapi/kernel/thread.h b/include/cobalt/uapi/kernel/thread.h
index 664def08ef..279a96e19e 100644
--- a/include/cobalt/uapi/kernel/thread.h
+++ b/include/cobalt/uapi/kernel/thread.h
@@ -73,6 +73,8 @@
 #define XNPIALERT 0x00000080 /**< Priority inversion alert (SIGDEBUG sent) */
 #define XNSCHEDP  0x00000100 /**< schedparam propagation is pending */
 #define XNCONTHI  0x00000200 /**< Continue in primary mode after debugging */
+#define XNPNDSUSP 0x00000400 /**< Pending suspension on migrate syscall */
+#define XNPNDHELD 0x00000800 /**< Pending held on migrate syscall */
 
 /* Local information flags (private to current thread) */
 
diff --git a/kernel/cobalt/posix/signal.c b/kernel/cobalt/posix/signal.c
index 1842aa5e8a..71277d7ca6 100644
--- a/kernel/cobalt/posix/signal.c
+++ b/kernel/cobalt/posix/signal.c
@@ -510,6 +510,7 @@ int __cobalt_kill(struct cobalt_thread *thread, int sig, int group) /* nklocked,
 			ret = -EINTR;
 		break;
 	case SIGRESM:
+		xnthread_clear_info(&thread->threadbase, XNPNDSUSP);
 		xnthread_resume(&thread->threadbase, XNSUSP);
 		goto resched;
 	case SIGRELS:
diff --git a/kernel/cobalt/posix/syscall.c b/kernel/cobalt/posix/syscall.c
index bc1825676f..7dca49a424 100644
--- a/kernel/cobalt/posix/syscall.c
+++ b/kernel/cobalt/posix/syscall.c
@@ -118,6 +118,8 @@ static void prepare_for_signal(struct task_struct *p,
 static COBALT_SYSCALL(migrate, current, (int domain))
 {
 	struct xnthread *thread = xnthread_current();
+	int err;
+	spl_t s;
 
 	if (is_secondary_domain()) {
 		if (domain == COBALT_PRIMARY) {
@@ -131,7 +133,29 @@ static COBALT_SYSCALL(migrate, current, (int domain))
 			if (xnthread_test_state(thread, XNDORMANT))
 				return 0;
 
-			return xnthread_harden() ? : 1;
+			err = xnthread_harden();
+			if (err)
+				return err;
+
+			xnlock_get_irqsave(&nklock, s);
+
+			if (xnthread_test_info(thread, XNPNDSUSP|XNPNDHELD)) {
+				int mask = 0;
+
+				if (xnthread_test_info(thread, XNPNDSUSP))
+					mask |= XNSUSP;
+				if (xnthread_test_info(thread, XNPNDHELD))
+					mask |= XNHELD;
+
+				xnthread_clear_info(thread,
+						    XNPNDSUSP|XNPNDHELD);
+				xnthread_suspend(thread, mask, XN_INFINITE,
+						 XN_RELATIVE, NULL);
+			}
+
+			xnlock_put_irqrestore(&nklock, s);
+
+			return 1;
 		}
 		return 0;
 	}
diff --git a/kernel/cobalt/sched-sporadic.c b/kernel/cobalt/sched-sporadic.c
index bec8dcb533..80e99a1e7d 100644
--- a/kernel/cobalt/sched-sporadic.c
+++ b/kernel/cobalt/sched-sporadic.c
@@ -152,9 +152,10 @@ retry:
 	if (pss->budget == 0)
 		return;
 
-	if (xnthread_test_state(thread, XNHELD))
+	if (xnthread_test_state(thread, XNHELD)) {
+		xnthread_clear_info(thread, XNPNDHELD);
 		xnthread_resume(thread, XNHELD);
-	else if (thread->cprio < pss->param.normal_prio) {
+	} else if (thread->cprio < pss->param.normal_prio) {
 		p.pss.init_budget = 0;
 		p.pss.current_prio = pss->param.normal_prio;
 		/* Move sporadic thread to the foreground. */
diff --git a/kernel/cobalt/thread.c b/kernel/cobalt/thread.c
index 60a7112844..b605781c64 100644
--- a/kernel/cobalt/thread.c
+++ b/kernel/cobalt/thread.c
@@ -1013,8 +1013,13 @@ void xnthread_suspend(struct xnthread *thread, int mask,
 	 * state, since they are suspended by definition.
 	 */
 	if (((oldstate & (XNTHREAD_BLOCK_BITS|XNUSER)) == (XNRELAX|XNUSER)) &&
-	    (mask & (XNDELAY | XNSUSP | XNHELD)) != 0)
+	    (mask & (XNDELAY | XNSUSP | XNHELD)) != 0) {
+		if (mask & XNSUSP)
+			xnthread_set_info(thread, XNPNDSUSP);
+		if (mask & XNHELD)
+			xnthread_set_info(thread, XNPNDHELD);
 		__xnthread_signal(thread, SIGSHADOW, SIGSHADOW_ACTION_HARDEN);
+	}
 out:
 	xnlock_put_irqrestore(&nklock, s);
 	return;
-- 
2.47.3