lkml.org 
[lkml]   [2020]   [Jan]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    SubjectRe: [PATCH v3 2/7] uaccess: Tell user_access_begin() if it's for a write or not
    From
    On January 23, 2020 11:57:57 AM PST, Linus Torvalds <torvalds@linux-foundation.org> wrote:
    >On Thu, Jan 23, 2020 at 11:47 AM christophe leroy
    ><christophe.leroy@c-s.fr> wrote:
    >>
    >> I'm going to leave it aside, at least for the time being, and do it
    >as a
    >> second step later after evaluating the real performance impact. I'll
    >> respin tomorrow in that way.
    >
    >Ok, good.
    >
    >From a "narrow the access window type" standpoint it does seem to be a
    >good idea to specify what kind of user accesses will be done, so I
    >don't hate the idea, it's more that I'm not convinced it matters
    >enough.
    >
    >On x86, we have made the rule that user_access_begin/end() can contain
    >_very_ few operations, and objtool really does enforce that. With
    >objtool and KASAN, you really end up with very small ranges of
    >user_access_begin/end().
    >
    >And since we actually verify it statically on x86-64, I would say that
    >the added benefit of narrowing by access type is fairly small. We're
    >not going to have complicated code in that user access region, at
    >least in generic code.
    >
    >> > Also, it shouldn't be a "is this a write". What if it's a read
    >_and_ a
    >> > write? Only a write? Only a read?
    >>
    >> Indeed that was more: does it includes a write. It's either RO or RW
    >
    >I would expect that most actual users would be RO or WO, so it's a bit
    >odd to have those choices.
    >
    >Of course, often writing ends up requiring read permissions anyway if
    >the architecture has problems with alignment handling or similar, but
    >still... The real RW case does exist conceptually (we have
    >"copy_in_user()", after all), but still feels like it shouldn't be
    >seen as the only _interface_ choice.
    >
    >IOW, an architecture may decide to turn WO into RW because of
    >architecture limitations (or, like x86 and arm, ignore the whole
    >RO/RW/WO _entirely_ because there's just a single "allow user space
    >accesses" flag), but on an interface layer if we add this flag, I
    >really think it should be an explicit "read or write or both".
    >
    >So thus my "let's try to avoid doing it in the first place, but if we
    >_do_ do this, then do it right" plea.
    >
    > Linus

    I'm wondering if we should make it a static part of the API instead of a variable.

    I have *deep* concern with carrying state in a "key" variable: it's a direct attack vector for a crowbar attack, especially since it is by definition live inside a user access region.

    One major reason x86 restricts the regions like this is to minimize the amount of unconstrained state: we don't save and restore the state around, but enter and exit unconditionally, which means that a leaked state will end up having a limited lifespan. Nor is there any state inside the user access region which could be corrupted to leave the region open.
    --
    Sent from my Android device with K-9 Mail. Please excuse my brevity.

    \
     
     \ /
      Last update: 2020-01-24 03:04    [W:2.603 / U:0.036 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site