lkml.org 
[lkml]   [2020]   [Jan]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [PATCH v3 2/7] uaccess: Tell user_access_begin() if it's for a write or not
    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

    \
     
     \ /
      Last update: 2020-01-23 20:59    [W:3.206 / U:0.004 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site