[RFC PATCH 02/16] sched/membarrier: Report only completed registrations

[Mathieu Desnoyers]

MEMBARRIER_CMD_GET_REGISTRATIONS returns the registration commands
which the process issued.

Each registration has two state bits: one is set when the registration
starts and the "ready" one when it completes. The command reports a
registration when either bit is set.

The sync core and rseq registrations also set
MEMBARRIER_STATE_PRIVATE_EXPEDITED, without its "ready" bit. They set
it because their commands need the same scheduler barriers as
MEMBARRIER_CMD_PRIVATE_EXPEDITED. So a process which only issued
MEMBARRIER_CMD_REGISTER_PRIVATE_EXPEDITED_RSEQ is reported as having
issued MEMBARRIER_CMD_REGISTER_PRIVATE_EXPEDITED as well. Yet
MEMBARRIER_CMD_PRIVATE_EXPEDITED returns -EPERM for that process.

A registration which failed with -ENOMEM would be reported the same
way.

User-space gets a wrong registration mask. The command was added for
CRIU, which saves the registrations of a process at checkpoint. A
restored process would carry a registration which the original
process did not issue. A program which trusts the mask would issue
MEMBARRIER_CMD_PRIVATE_EXPEDITED and get -EPERM.

Fix this by reporting a registration only when its "ready" bit is
set.

This changes the mask which user-space reads.
MEMBARRIER_CMD_GET_REGISTRATIONS no longer reports
MEMBARRIER_CMD_REGISTER_PRIVATE_EXPEDITED for a process which only
issued the sync core or the rseq registration. The result of every
other command is unchanged.

Fixes: 544a4f2ecd45 ("sched/membarrier: Introduce MEMBARRIER_CMD_GET_REGISTRATIONS")
Cc: stable at vger.kernel.org
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers at efficios.com>
---
 kernel/sched/membarrier.c | 29 ++++++++++++++---------------
 1 file changed, 14 insertions(+), 15 deletions(-)

diff --git a/kernel/sched/membarrier.c b/kernel/sched/membarrier.c
index cb957b8f1946..46e823a3d668 100644
--- a/kernel/sched/membarrier.c
+++ b/kernel/sched/membarrier.c
@@ -566,15 +566,17 @@ static int membarrier_get_registrations(void)
 	struct task_struct *p = current;
 	struct mm_struct *mm = p->mm;
 	int registrations_mask = 0, membarrier_state, i;
-	static const int states[] = {
-		MEMBARRIER_STATE_GLOBAL_EXPEDITED |
-			MEMBARRIER_STATE_GLOBAL_EXPEDITED_READY,
-		MEMBARRIER_STATE_PRIVATE_EXPEDITED |
-			MEMBARRIER_STATE_PRIVATE_EXPEDITED_READY,
-		MEMBARRIER_STATE_PRIVATE_EXPEDITED_SYNC_CORE |
-			MEMBARRIER_STATE_PRIVATE_EXPEDITED_SYNC_CORE_READY,
-		MEMBARRIER_STATE_PRIVATE_EXPEDITED_RSEQ |
-			MEMBARRIER_STATE_PRIVATE_EXPEDITED_RSEQ_READY
+	/*
+	 * Only the "ready" state tells that a registration command
+	 * completed. The other state of each pair is not specific to
+	 * one registration command: the sync core and rseq
+	 * registrations also set MEMBARRIER_STATE_PRIVATE_EXPEDITED.
+	 */
+	static const int ready_states[] = {
+		MEMBARRIER_STATE_GLOBAL_EXPEDITED_READY,
+		MEMBARRIER_STATE_PRIVATE_EXPEDITED_READY,
+		MEMBARRIER_STATE_PRIVATE_EXPEDITED_SYNC_CORE_READY,
+		MEMBARRIER_STATE_PRIVATE_EXPEDITED_RSEQ_READY
 	};
 	static const int registration_cmds[] = {
 		MEMBARRIER_CMD_REGISTER_GLOBAL_EXPEDITED,
@@ -582,16 +584,13 @@ static int membarrier_get_registrations(void)
 		MEMBARRIER_CMD_REGISTER_PRIVATE_EXPEDITED_SYNC_CORE,
 		MEMBARRIER_CMD_REGISTER_PRIVATE_EXPEDITED_RSEQ
 	};
-	BUILD_BUG_ON(ARRAY_SIZE(states) != ARRAY_SIZE(registration_cmds));
+	BUILD_BUG_ON(ARRAY_SIZE(ready_states) != ARRAY_SIZE(registration_cmds));
 
 	membarrier_state = atomic_read(&mm->membarrier_state);
-	for (i = 0; i < ARRAY_SIZE(states); ++i) {
-		if (membarrier_state & states[i]) {
+	for (i = 0; i < ARRAY_SIZE(ready_states); ++i) {
+		if (membarrier_state & ready_states[i])
 			registrations_mask |= registration_cmds[i];
-			membarrier_state &= ~states[i];
-		}
 	}
-	WARN_ON_ONCE(membarrier_state != 0);
 	return registrations_mask;
 }
 
-- 
2.47.3