On January 23, 2020 11:57:57 AM PST, Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> wrote: >On Thu, Jan 23, 2020 at 11:47 AM christophe leroy ><christophe.leroy@xxxxxx> 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.
