Thread (43 messages) 43 messages, 7 authors, 2018-07-03

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

From: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Date: 2018-07-03 17:26:49
Also in: lkml

----- On Jul 3, 2018, at 1:10 PM, Linus Torvalds torvalds@linux-foundation.org wrote:
On Tue, Jul 3, 2018 at 9:40 AM Andi Kleen [off-list ref] wrote:
quoted
So it sounds like architectures that don't have an instruction atomic u64
*_user need to disable interrupts during the access, and somehow handle that
case when a page fault happens?
No. It's actually the store by *user* space that is the critical one.
Not the whole 64-bit value, just the low pointer part.

The kernel could do it as a byte-by-byte load, really. It's
per-thread, and once the kernel is running, it's not going to change.
The kernel never changes the value, it just loads it from user space.

So all the atomicity worries for the kernel are a red herring. They'd
arguably be nice to have - but only for an insane case that makes
absolutely no sense (a different thread trying to change the value).

Can we please stop the idiocy already? The kernel could read the rseq
pointer one bit at a time, and do a little dance with "yield()" in
between, and take interrupts and page faults, and it wouldn't matter
AT ALL.

It's not even that we read the value from an interrupt context, it's
that as we return to user space (which can be the result of an
interrupt) we can read the value.

This whole thread has been filled with crazy "what if" things that don't matter.
Sorry to come back in the thread late, looks like I've missed all the
fun.

I agree with Linus: we can simply document that updates to rseq->rseq_cs
should be thread-local in the rseq uapi and be done with it. This would
allow using get_user(u64) even on 32-bit architectures, because we cannot
care less if an architecture chooses to read the u64 byte-wise while
standing on its feet.

With this added requirement, Andy's idea of using a union between __u64
and upper/lower __u32 would fit very nicely.

If everyone is OK with that approach, I can prepare an updated patch.

Thanks,

Mathieu

-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help