Commit 0190e419 authored by Mathieu Desnoyers's avatar Mathieu Desnoyers Committed by Ingo Molnar
Browse files

rseq: Deprecate RSEQ_CS_FLAG_NO_RESTART_ON_* flags 

The pretty much unused RSEQ_CS_FLAG_NO_RESTART_ON_* flags introduce
complexity in rseq, and are subtly buggy [1]. Solving those issues
requires introducing additional complexity in the rseq implementation
for each supported architecture.

Considering that it complexifies the rseq ABI, I am proposing that we
deprecate those flags. [2]

So far there appears to be consensus from maintainers of user-space
projects impacted by this feature that its removal would be a welcome
simplification. [3]

The deprecation approach proposed here is to issue WARN_ON_ONCE() when
encountering those flags and kill the offending process with sigsegv.
This should allow us to quickly identify whether anyone yells at us for
removing this.

Link: https://lore.kernel.org/lkml/20220618182515.95831-1-minhquangbui99@gmail.com/ [1]
Link: https://lore.kernel.org/lkml/258546133.12151.1655739550814.JavaMail.zimbra@efficios.com/ [2]
Link: https://lore.kernel.org/lkml/87pmj1enjh.fsf@email.froward.int.ebiederm.org/

 [3]
Signed-off-by: default avatarMathieu Desnoyers <mathieu.desnoyers@efficios.com>
Signed-off-by: default avatarPeter Zijlstra (Intel) <peterz@infradead.org>
Signed-off-by: default avatarIngo Molnar <mingo@kernel.org>
Link: https://lore.kernel.org/lkml/20220622194617.1155957-1-mathieu.desnoyers@efficios.com
parent 91caa5ae

kernel/rseq.c

+8 −15
Original line number Diff line number Diff line
@@ -18,8 +18,9 @@ 
#define CREATE_TRACE_POINTS

#include <trace/events/rseq.h>



#define RSEQ_CS_PREEMPT_MIGRATE_FLAGS (RSEQ_CS_FLAG_NO_RESTART_ON_MIGRATE | \

				       RSEQ_CS_FLAG_NO_RESTART_ON_PREEMPT)

#define RSEQ_CS_NO_RESTART_FLAGS (RSEQ_CS_FLAG_NO_RESTART_ON_PREEMPT | \

				  RSEQ_CS_FLAG_NO_RESTART_ON_SIGNAL | \

				  RSEQ_CS_FLAG_NO_RESTART_ON_MIGRATE)



/*

 *
@@ -175,23 +176,15 @@ static int rseq_need_restart(struct task_struct *t, u32 cs_flags) 
	u32 flags, event_mask;

	int ret;



	if (WARN_ON_ONCE(cs_flags & RSEQ_CS_NO_RESTART_FLAGS))

		return -EINVAL;



	/* Get thread flags. */

	ret = get_user(flags, &t->rseq->flags);

	if (ret)

		return ret;



	/* Take critical section flags into account. */

	flags |= cs_flags;



	/*

	 * Restart on signal can only be inhibited when restart on

	 * preempt and restart on migrate are inhibited too. Otherwise,

	 * a preempted signal handler could fail to restart the prior

	 * execution context on sigreturn.

	 */

	if (unlikely((flags & RSEQ_CS_FLAG_NO_RESTART_ON_SIGNAL) &&

		     (flags & RSEQ_CS_PREEMPT_MIGRATE_FLAGS) !=

		     RSEQ_CS_PREEMPT_MIGRATE_FLAGS))

	if (WARN_ON_ONCE(flags & RSEQ_CS_NO_RESTART_FLAGS))

		return -EINVAL;



	/*
@@ -203,7 +196,7 @@ static int rseq_need_restart(struct task_struct *t, u32 cs_flags) 
	t->rseq_event_mask = 0;

	preempt_enable();



	return !!(event_mask & ~flags);

	return !!event_mask;

}



static int clear_rseq_cs(struct task_struct *t)