Re: [PATCH] rseq: x86: Fix rseq_cs get cleared when returning from signal handler

From: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
To: "Eric W. Biederman" <ebiederm@xmission.com>
Cc: Derek Bruening <bruening@google.com>,
	Kevin Malachowski <chowski@google.com>,
	Alexander Mikhalitsyn <alexander@mihalicyn.com>,
	Florian Weimer <fw@deneb.enyo.de>,
	Carlos O'Donell <carlos@redhat.com>, Paul Turner <pjt@google.com>,
	Simon Marchi <simon.marchi@efficios.com>,
	Peter Oskolkov <posk@posk.io>,
	Chris Kennelly <ckennelly@google.com>,
	Pedro Alves <palves@redhat.com>,
	Bui Quang Minh <minhquangbui99@gmail.com>,
	linux-kernel <linux-kernel@vger.kernel.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
	Dave Hansen <dave.hansen@linux.intel.com>, x86 <x86@kernel.org>,
	"H. Peter Anvin" <hpa@zytor.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Juri Lelli <juri.lelli@redhat.com>,
	Vincent Guittot <vincent.guittot@linaro.org>,
	Dietmar Eggemann <dietmar.eggemann@arm.com>,
	Steven Rostedt <rostedt@goodmis.org>,
	Ben Segall <bsegall@google.com>, Mel Gorman <mgorman@suse.de>,
	Daniel Bristot de Oliveira <bristot@redhat.com>,
	Valentin Schneider <vschneid@redhat.com>,
	"Paul E. McKenney" <paulmck@kernel.org>,
	Boqun Feng <boqun.feng@gmail.com>,
	Kees Cook <keescook@chromium.org>,
	"Chang S. Bae" <chang.seok.bae@intel.com>,
	Brian Gerst <brgerst@gmail.com>, Andrei Vagin <avagin@gmail.com>,
	Pavel Tikhomirov <ptikhomirov@virtuozzo.com>
Subject: Re: [PATCH] rseq: x86: Fix rseq_cs get cleared when returning from signal handler
Date: Tue, 21 Jun 2022 16:05:12 -0400 (EDT)	[thread overview]
Message-ID: <871717002.20576.1655841912053.JavaMail.zimbra@efficios.com> (raw)
In-Reply-To: <87y1xper17.fsf@email.froward.int.ebiederm.org>

----- On Jun 21, 2022, at 3:48 PM, Eric W. Biederman ebiederm@xmission.com wrote:

> Derek Bruening <bruening@google.com> writes:
> 
>> From the viewpoint of dynamic binary translation/instrumentation and
>> memtrace (go/memtrace), removing those RSEQ_CS_FLAG_NO_RESTART_ON_* flags
>> is a good thing as it reduces complexity and makes it easier to handle rseq
>> (which is painful enough to handle already).
> 
> It sounds like there is consensus.
> 
> Does someone want to code up a simple patch that detects when
> RSEQ_CS_NO_RESTART_ON_SIGNAL and does a WARN_ON_ONCE and fails if
> someone uses so it can be set to Linus in the next merge window.
> 
> After no one screams at that patch it should be safe to remove the
> functionality, because you have empirical proof that no one uses
> that functionality.

Sure, I can whip up something.

I'll send it to Peter Zijlstra shortly.

I plan to, as you suggest, WARN_ON_ONCE() when this happens, and return
an error when the rseq flags or rseq_cs flags contain either of the
RSEQ_CS_FLAG_NO_RESTART_ON_* flags. This error is handled by forcing a
killing the process with sigsegv:

__rseq_handle_notify_resume()
[...]
error:
        sig = ksig ? ksig->sig : 0;
        force_sigsegv(sig);

Does it look acceptable ?

Thanks,

Mathieu

-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com