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

From: Eric W. Biederman
Date: Tue Jun 21 2022 - 17:19:24 EST


Mathieu Desnoyers <mathieu.desnoyers@xxxxxxxxxxxx> writes:

> ----- On Jun 21, 2022, at 3:48 PM, Eric W. Biederman ebiederm@xxxxxxxxxxxx wrote:
>
>> Derek Bruening <bruening@xxxxxxxxxx> 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 ?

I think so. force_sigsegv preps things so that when you go into
exit_to_user_mode_loop the signal handler kills the process.

So assuming that happens after force_sigsegv that looks good.

Eric