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