lkml.org 
[lkml]   [2018]   [Jul]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [RFC PATCH for 4.18] rseq: use __u64 for rseq_cs fields, validate user inputs
    ----- 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 <andi@firstfloor.org> wrote:
    >>
    >> 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

    \
     
     \ /
      Last update: 2018-07-03 19:27    [W:2.410 / U:0.308 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site