Re: [RFC PATCH for 4.18] rseq: use __u64 for rseq_cs fields, validate user inputs

On Tue, Jul 03, 2018 at 10:55:46AM +0200, Heiko Carstens wrote:

quoted

quoted

quoted

We're piece-wise enabling rseq across architectures anyway, and when the
relevant maintains do this, they can have a look at their
{get,put}_user() implementations and fix them.

If you rely on get_user(u64) working, that means microblaze is already
broken, but I suppose it already was, since their rseq enablement patch
is extremely dodgy. Michal?

s390 uses the mvcos instruction to implement get_user(). That instruction
is not defined to be atomic, but may copy bytes piecemeal.. I had the
impression that the rseq fields are supposed to be updated within the
context of a single thread (user + kernel space).

However if another user space thread is allowed to do this as well, then
the get_user() approach won't fly on s390.

That leaves the question: does it even make sense for a thread to update
the rseq structure of a different thread?

The problem is interrupts; we need interrupts on the CPU doing the store
to observe either the old or the new value, not a mix.

If mvcos does not guarantee that, we're having problems. Is there a
reason get_user() cannot use a 'regular' load?

Well, that's single instruction semantics. This is something we actually
can guarantee, since the mvcos instruction itself won't be interrupted and
copies all 1/2/4/8 bytes in a row.

So we are talking about that single instructions are required and not
atomic accesses?

And to answer also your question: we don't use a regular load, since we
would have to use 'sacf' construct surrounding the load instruction which
would be much slower.
We have something like that implemented for the futex atomic ops, and we
could also implement something like that for this use case
(e.g. get_user_atomic()), if really needed.