On Thu, Sep 23, 2021 at 11:13 AM Paolo Bonzini <pbonzini@xxxxxxxxxx> wrote:
>
> A fix for a bug with restartable sequences and KVM. KVM's handling
> of TIF_NOTIFY_RESUME, e.g. for task migration, clears the flag without
> informing rseq and leads to stale data in userspace's rseq struct.
Ok, patches look reasonable.
> I'm sending this as a separate pull request since it's not code
> that I usually touch. In particular, patch 2 ("entry: rseq: Call
> rseq_handle_notify_resume() in tracehook_notify_resume()") is just a
> cleanup to try and make future bugs less likely. If you prefer this to
> be sent via Thomas and only in 5.16, please speak up.
So I took the pull request this way, thanks for separating it like this.
But I'm adding a few people to the cc for a completely different
reason: the cleanup to move all the notify_resume stuff to
tracehook_notify_resume() is good, but it does make me go - once again
- "Hmm, that naming is really really bad".
The <linux/tracehook.h> code was literally meant for tracing. It's
where the name comes from, and it's the original intent: having a
place that you can hook into for tracing that doesn't depend on how
the core kernel code ends up changing.
But that's not how it actually acts right now. That header file is now
some very core functionality, and little of it is actually related to
tracing any more. It's more core process state handling for the user
space return path.
So I don't object to the patches, and they are merged, but I'm cc'ing people to
(a) let them know about this (see commit a68de80f61f6: "entry: rseq:
Call rseq_handle_notify_resume() in tracehook_notify_resume()" in the
current -git tree)
(b) possibly prod some people into perhaps moving/renaming some of
that code to actual core kernel C files, instead of a misnamed header
file..
Hmm?
Linus
