Re: [RESEND][PATCH 3/3] rseq: Ensure SIGBUS delivered on memory failure

[Mathieu Desnoyers <mathieu.desnoyers@efficios.com>]

On 2024-08-06 03:51, Peter Zijlstra wrote:
> On Mon, Aug 05, 2024 at 09:37:48PM -0700, Kees Cook wrote:
>> On Tue, Jul 23, 2024 at 04:47:52PM +0200, Andrew Zaborowski wrote:
>>> Uncorrected memory errors for user pages are signaled to processes
>>> using SIGBUS or, if the error happens in a syscall, an error retval
>>> from the syscall.  The SIGBUS is documented in
>>> Documentation/mm/hwpoison.rst#failure-recovery-modes
>>>
>>> Once a user task sets t->rseq in the rseq() syscall, if the kernel
>>> cannot access the memory pointed to by t->rseq->rseq_cs, that initial
>>> rseq() and all future syscalls should return an error so understandably
>>> the code just kills the task.
>>>
>>> To ensure that SIGBUS is used set the new t->kill_on_efault flag and
>>> run queued task work on rseq_get_rseq_cs() errors to give memory_failure
>>> the chance to run.
>>>
>>> Note: the rseq checks run inside resume_user_mode_work() so whenever
>>> _TIF_NOTIFY_RESUME is set.  They do not run on every syscall exit so
>>> I'm not concerned that these extra flag operations are in a hot path,
>>> except with CONFIG_DEBUG_RSEQ.
>>>
>>> Signed-off-by: Andrew Zaborowski <andrew.zaborowski@intel.com>
>>> ---
>>>   kernel/rseq.c | 25 +++++++++++++++++++++----
>>
>> Can an rseq maintainer please review this? I can carry it via the execve
>> tree with the related patches...
> 
> *sigh*,.. because get_maintainers just doesn't work or something?
> 
> Anyway, I'm confused by the signal code (as always), why isn't the
> task_work_run() in get_signal() sufficient?
> 
> At some point we're going to run into trouble with sprinkling
> task_work_run() around willy nilly :/

I agree with Peter: adding explicit calls to task_work_run all over
the kernel does not appear to be an elegant solution.

One thing I am missing is a clear motivation for adding this code:
what is the real-world use-case that benefits from getting this SIGBUS
rather than a SIGSEGV or -EFAULT ?

Also, I feel like we should investigate turning SIGSEGV into SIGBUS
at signal delivery rather than handle this here and there in the kernel
code.

Thoughts ?

Thanks,

Mathieu


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